- 22 3月, 2019 32 次提交
-
-
由 Eric Blake 提交于
Now that the core of SnapshotObj is agnostic to snapshots and can be shared with upcoming checkpoint code, it is time to rename the struct and the functions specific to list operations. A later patch will shuffle which file holds the common code. This is a fairly mechanical patch. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Another step towards making the object list reusable for both snapshots and checkpoints: the list code only ever needs items that are in the common virDomainMomentDef base type. This undoes a lot of the churn in accessing common members added in the previous patch, and the bulk of the patch is mechanical. But there was one spot where I had to unroll a VIR_STEAL_PTR to work around changed types. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Pull out the common parts of virDomainSnapshotDef that will be reused for virDomainCheckpointDef into a new base class. Adjust all callers that use the direct fields (some of it is churn that disappears when the next patch refactors virDomainSnapshotObj; oh well...). Someday, I hope to switch this type to be a subclass of virObject, but that requires a more thorough audit of cleanup paths, and besides minimal incremental changes are easier to review. As for the choice of naming: I promised my teenage daughter Evelyn that I'd give her credit for her contribution to this commit. I asked her "What would be a good name for a base class for DomainSnapshot and DomainCheckpoint". After explaining what a base class was (using the classic OOB Square and Circle inherit from Shape), she came up with "DomainMoment", which is way better than my initial thought of "DomainPointInTime" or "DomainPIT". Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Separate the algorithm for which list members to vist (which is generic and can be shared with checkpoints, provided that common filtering bits are either declared with the same value or have a mapping from public API to common value) from the decision on which members to return (which is specific to snapshots). The typedef for the callback function feels a bit heavy here, but will make it easier to move the common portions in a later patch. As part of the refactoring, note that the macros for selecting filter bits are specific to listing functionality, so they belong better in virdomainsnapshotobjlist.h (missed in commit 9b75154c). Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
An upcoming patch will rework virDomainSnapshotObjList to be generic for both snapshots and checkpoints; reduce the churn by adding a new accessor virDomainSnapshotObjGetDef() which returns the snapshot-specific definition even when the list is rewritten to operate only on a base class, then using it at sites that that are specific to snapshots. Use VIR_STEAL_PTR when appropriate in the affected lines. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Rather than allowing a leaky abstraction where multiple drivers have to open-code operations that update the relations in a virDomainSnapshotObjList, it is better to add accessor functions so that updates to relations are maintained closer to the internals. This patch finishes the job started in the previous patch, by getting rid of all direct access to nchildren, first_child, or sibling outside of the lowest level functions, making it easier to refactor later on. The lone new caller to virDomainSnapshotObjListSize() checks for a return != 0, because it wants to handles errors (-1, only possible if the hash table wasn't allocated) and existing snapshots (> 0) in the same manner; we can drop the check for a current snapshot on the grounds that there shouldn't be one if there are no snapshots. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Rather than allowing a leaky abstraction where multiple drivers have to open-code operations that update the relations in a virDomainSnapshotObjList, it is better to add accessor functions so that updates to relations are maintained closer to the internals. This patch starts the task with a single new function: virDomainSnapshotMoveChildren(). The logic might not be immediately obvious [okay, that's an understatement - the existing code uses black magic ;-)], so here's an overview: The old code has an implicit for loop around each call to qemuDomainSnapshotReparentChildren() by using virDomainSnapshotForEachChild() (you'll need a wider context than git's default of 3 lines to see that); the new code has a more visible for loop. Then it helps if you realize that the code is making two separate changes to each child object: STRDUP of the new parent name prior to writing XML files (unchanged), and touching up the pointer to the parent object (refactored); the end result is the same whether a single pass made both changes (both in driver code), or whether it is split into two passes making one change each (one in driver code, the other in the new accessor). Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
It is easier to track the current snapshot as part of the list of snapshots. In particular, doing so lets us guarantee that the current snapshot is cleared if that snapshot is removed from the list (rather than depending on the caller to do so, and risking a use-after-free problem, such as the one recently patched in 1db9d0ef). This requires the addition of several new accessor functions, as well as a useful return type for virDomainSnapshotObjListRemove(). A few error handling sites that were previously setting vm->current_snapshot = NULL can now be dropped, because the previous function call has now done it already. Also, qemuDomainRevertToSnapshot() was setting the current vm twice, so keep only the one used on the success path. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Rework the logic in qemuDomainSnapshotLoad() to set vm->current_snapshot only once at the end of the loop, rather than repeatedly querying it during the loop, to make it easier for the next patch to use accessor functions rather than direct manipulation of vm->current_snapshot. When encountering multiple snapshots claiming to be current (based on the presence of an <active>1</active> element in the XML, which libvirt only outputs for internal use and not for any public API), this changes behavior from warning only once and running with no current snapshot, to instead warning on each duplicate and selecting the last one encountered (which is arbitrary based on readdir() ordering, but actually stands a fair chance of being the most-recently created snapshot whether by timestamp or by the propensity of humans to name things in ascending order). Note that the code in question is only run by libvirtd when it first starts, reading state from disk from the previous run into memory for this run. Since the data resides somewhere that only libvirt should be touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be clean. So in the common case, the code touched here is unreachable. But if someone is actually messing with files behind libvirt's back, they deserve the change in behavior. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
The only use for the 'current' member of virDomainSnapshotDef was with the PARSE/FORMAT_INTERNAL flag for controlling an internal-use <active> element marking whether a particular snapshot definition was current, and even then, only by the qemu driver on output, and by qemu and test driver on input. But this duplicates vm->snapshot_current, and gets in the way of potential simplifications to have qemu store a single file for all snapshots rather than one file per snapshot. Get rid of the member by adding a bool* parameter during parse (ignored if the PARSE_INTERNAL flag is not set), and by adding a new flag during format (if FORMAT_INTERNAL is set, the value printed in <active> depends on the new FORMAT_CURRENT). Then update the qemu driver accordingly, which involves hoisting assignments to vm->current_snapshot to occur prior to any point where a snapshot XML file is written (although qemu kept vm->current_snapshot and snapshot->def_current in sync by the end of the function, they were not always identical in the middle of functions, so the shuffling gets a bit interesting). Later patches will clean up some of that confusing churn to vm->current_snapshot. Note: even if later patches refactor qemu to no longer use FORMAT_INTERNAL for output (by storing bulk snapshot XML instead), we will always need PARSE_INTERNAL for input (because on upgrade, a new libvirt still has to parse XML left from a previous libvirt). Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
When a future patch converts virDomainSnapshotDef to be a virObject, we need to be careful that converting VIR_FREE() to virObjectUnref() does not result in double frees. Reorder the assignment of def into the object to the point after object is in the hash table (as otherwise the virHashAddEntry() error path would have a shot at freeing def prematurely). Suggested-by: NJohn Ferlan <ferlan@redhat.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
由 Eric Blake 提交于
Change the return value of virDomainSnapshotObjListParse() to return the number of snapshots imported, and allow a return of 0 (the original proposal of adding a flag to virDomainSnapshotCreateXML required returning an arbitrary non-NULL snapshot, but that idea was abandoned; and by returning a count, we are no longer constrained to a non-empty list). Document which flags are supported (namely, just SECURE) in virDomainSnapshotObjListFormat(). Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
An upcoming patch will be reworking virDomainSnapshotDef to have a base class; minimize the churn by using a local variable to reduce the number of dereferences required when acessing the domain definition associated with the snapshot. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Cole Robinson 提交于
Move DO_TEST* qemuCaps init into testInfoSetArgs. This is a step towards unifying the different test macro implementations Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This is closer to the pattern of qemuxml2xml tests, and will make things easier if we extend testInfo to contain more freeable data Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Rather than make callers do it. The operative info is just arch and ver which we are passing in already. Fold in stripmachinealiases too since it is just dependent on ver value Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Only initialize the fields that are passed in Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
They are potentially useful at the moment, but we will be making things much more flexible Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
It only has one caller. Just use DO_TEST_FULL Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This allows us to drop parseFlags from DO_TEST_FULL Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This allows us to drop flags from DO_TEST_FULL Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This allows us to drop migrateFrom and migrateFd from DO_TEST_FULL Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This allows us to drop stub GIC values from DO_TEST_FULL calls Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This is necessary before we can start adding more optional parameter implementations to DO_TEST_FULL Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
This establishes a pattern that will allow us to make test macros more general purpose, by taking optional arguments. The general format will be: DO_TEST_FULL(... ARG_FOO, <value1>, ARG_BAR, <value2>) ARG_X are just enum values that we look for in the va_args and know how to interpret. Implement this for the existing implicit qemuCaps va_args Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
For now it just fills in the qemuCaps list. We will expand it in future patches Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
And adjust virQEMUCapsSetList to use it. It will also be used in future patches. Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
- 21 3月, 2019 2 次提交
-
-
由 Eric Blake 提交于
The following virsh command was triggering a use-after-free: $ virsh -c test:///default ' snapshot-create-as test s1 snapshot-create-as test s2 snapshot-delete --children-only test s1 snapshot-current --name test' Domain snapshot s1 created Domain snapshot s2 created Domain snapshot s1 children deleted error: name in virGetDomainSnapshot must not be NULL I got lucky on that run - although the error message is quite unexpected. On other runs, I was able to get a core dump, and valgrind confirms there is a definitive problem. The culprit? We were inconsistent about whether we set vm->current_snapshot, snap->def->current, or both when updating how the current snapshot was being tracked. As a result, deletion did not see that snapshot s2 was previously current, and failed to update vm->current_snapshot, so that the next API using the current snapshot failed because it referenced stale memory for the now-gone s2 (instead of the intended s1). The test driver code was copied from the qemu code (which DOES track both pieces of state everywhere), but was purposefully simplified because the test driver does not have to write persistent snapshot state to the file system. But when you realize that the only reason snap->def->current needs to exist is when writing out one file per snapshot for qemu, it's just as easy to state that the test driver never has to mess with the field (rather than chasing down which places forgot to set the field), and have vm->current_snapshot be the sole source of truth in the test driver. Ideally, I'd get rid of the 'current' member in virDomainSnapshotDef, as well as the 'current_snapshot' member in virDomainDef, and instead track the current member in virDomainSnapshotObjList, coupled with writing ALL snapshot state for qemu in a single file (where I can use <snapshots current='...'> as a wrapper, rather than VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL to output <current>1</current> XML on a per-snapshot file basis). But that's a bigger change, so for now I'm just patching things to avoid the test driver segfault. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Daniel P. Berrangé 提交于
We previously had to disable RBD on 32-bit platforms since Ceph has dropped all support for 32-bit. Unfortunately anyone with the RPM libvirt-daemon-driver-storage-rbd installed on 32-bit now has a broken upgrade path. To fix this we must make libvirt-daemon-driver-storage-core have an Obsoletes: libvirt-daemon-driver-storage-rbd < $VER-$REL Reviewed-by: NErik Skultety <eskultet@redhat.com> Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
-
- 20 3月, 2019 6 次提交
-
-
由 Michal Privoznik 提交于
https://bugzilla.redhat.com/show_bug.cgi?id=1686927 When trying to create a nwfilter binding via nwfilterBindingCreateXML() we may encounter a crash. The sequence of functions called is as follows: 1) nwfilterBindingCreateXML() parses the XML and calls virNWFilterBindingObjListAdd() which calls virNWFilterBindingObjListAddLocked() 2) Here, @binding is not found because binding->remove is set. 3) Therefore, controls continue with creating new @binding, setting its def to the one from 1) and adding it to the hash table. 4) This fails, because the binding is still in the hash table (duplicate key is detected). 5) The control jumps to 'error' label where virNWFilterBindingObjEndAPI() is called which frees the binding definition passed. 6) Error is propagated to the caller, which calls virNWFilterBindingDefFree() over the definition again. The solution is to unset binding->def in case of failure so it's not freed in step 5). Signed-off-by: NMichal Privoznik <mprivozn@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Peter Krempa 提交于
Storage source private data can be parsed along with other components of private data rather than a separate function which is called from multiple places. Signed-off-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Peter Krempa 提交于
virDomainDiskSourcePrivateDataParse and virDomainDiskSourcePRParse don't need the 'cleanup' label any more thanks to VIR_XPATH_NODE_AUTORESTORE. Signed-off-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Peter Krempa 提交于
The function does not have any code in the 'cleanup' label so we can simplify the control flow. Signed-off-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Peter Krempa 提交于
Get rid of the 'cleanup' label. Signed-off-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Peter Krempa 提交于
Signed-off-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-