- 26 3月, 2019 9 次提交
-
-
由 Laine Stump 提交于
Back in the bad old days different device types required a different qemu monitor call to detach them, and so an <interface type='hostdev'> needed to call the function for detaching hostdevs, while other <interface> types could be deleted as netdevs. Times have changed, and *all* device types are detached by calling the common function qemuDomainDeleteDevice(vm, alias), so we don't need to differentiate between hostdev interfaces and the others for that reason. There are a few other netdev-specific functions called during qemuDomainDetachNetDevice() (clearing bandwidth limits, stopping the interface), but those turn into NOPs when type=hostdev, so they're safe to call for type=hostdev. The only thing that is different + not a NOP is the call to virDomainAudit*() when qemuDomainDeleteDevice() fails, so if we add a conditional for that small bit of code, we can eliminate the callout from qemuDomainDetachNetDevice() to qemuDomainDetachThisDevice(), which makes this function fit the desired pattern for merging with the other detach functions, and paves the way to simplifying qemuDomainDetachHostDevice() too. Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com>
-
由 Laine Stump 提交于
qemuDomainDetachDiskDevice() is only called from one place. Moving the contents of the function to that place makes qemuDomainDetachDiskLive() more similar to the other Detach functions called by the toplevel qemuDomainDetachDevice(). The goal is to make each of the device-type-specific functions do this: 1) find the exact device 2) do any device-specific validation 3) do general validation 4) do device-specific shutdown (only needed for net devices) 5) do the common block of code to send device_del to qemu, then optionally wait for a corresponding DEVICE_DELETED event from qemu. with the final aim being that only items 1 & 2 will remain in each device-type-specific function, while 3 & 5 (which are the same for almost every type) will be de-duplicated and moved to the toplevel function that calls all of these (qemuDomainDetachDeviceLive(), which will also contain a callout to the one instance of (4) (netdev). Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com>
-
由 Laine Stump 提交于
qemuDomainDetachHostDevice() has a check at the end that calls qemuDomainDetachNetDevice() in the case that the hostdev is actually a Net device of type='hostdev'. A long time ago when device removal was (supposedly but not actually) synchronous, this would cause some extra code to be run prior to removing the device (e.g. restoring the original MAC address of the device, undoing some sort of virtual port profile, etc). For quite awhile now the device removal has been asynchronous, so that "extra teardown" isn't handled by the detach function, but instead is handled by the Remove function called at a later time. The result is that when we call qemuDomainDetachNetDevice() from qemuDomainDetachHostDevice(), it ends up just calling qemuDomainDetachThisHostDevice() and returning, which is exactly what we do for all other hostdevs anyway. Based on that, remove the behavioral difference when parent.type == VIR_DOMAIN_DEVICE_NET, and just call qemuDomainDetachThisHostDevice() for all hostdevs. Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com>
-
由 Laine Stump 提交于
There are separate Detach functions for PCI, USB, SCSI, Vhost, and Mediated hostdevs, but the functions are all 100% the same code, except that the PCI function checks for the guest side of the device being a PCI Multifunction device, while the other 4 check that the device's alias != NULL. The check for multifunction PCI devices should be done for *all* devices that are connected to the PCI bus in the guest, not just PCI hostdevs, and qemuIsMultiFunctionDevice() conveniently returns false if the queried device doesn't connect with PCI, so it is safe to make this check for all hostdev devices. (It also needs to be done for many other device types, but that will be addressed in a future patch). Likewise, since all hostdevs are detached by calling qemuDomainDeleteDevice(), which requires the device's alias, checking for a valid alias is a reasonable thing for PCI hostdevs too. Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Laine Stump 提交于
Having an InfoPtr named "dev" made my brain hurt. Renaming it to "info" gives one less thing to confuse when looking at the code. Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com>
-
由 Laine Stump 提交于
When support for hotplug/unplug of SCSI controllers was added way back in December 2009 (commit da9d937b), unplug was handled by calling the now-extinct function qemuMonitorRemovePCIDevice(), which required a PCI address as an argument. At the same time, the idea of every device in the config having a PCI address apparently was not yet fully implemented, because the author of the patch including a check for a valid PCI address in the device object. These days, all PCI devices are guaranteed to have a valid PCI address. But more important than that, we no longer detach devices by PCI address, but instead use qemuDomainDeleteDevice(), which identifies the device by its alias. So checking for a valid PCI address is just pointless extra code that obscures the high level of similarity between all the individual qemuDomainDetach*Device() functions. Signed-off-by: NLaine Stump <laine@laine.org> ACKed-by: NPeter Krempa <pkrempa@redhat.com> Reviewed-by: NJán Tomko <jtomko@redhat.com>
-
由 Laine Stump 提交于
qemuDomainRemoveRNGDevice() calls qemuDomainDetachExtensionDevice(). According to commit 1d1e264f that added this code, it should not be necessary to explicitly remove the zPCI extension device for a PCI device during unplug, because "QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way". In fact, no other devices call qemuDomainDetachExtensionDevice() during their qemuDomainRemove*Device() function, so it should be removed from qemuDomainRemoveRNGDevice as well. Signed-off-by: NLaine Stump <laine@laine.org> Reviewed-by: NJán Tomko <jtomko@redhat.com> Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
-
由 Laine Stump 提交于
qemuDomainDetachControllerDevice() calls qemuDomainDetachExtensionDevice() when the controller type is PCI. This is incorrect in multiple ways: * Any code that tears down a device should be in the qemuDomainRemove*Device() function (which is called after libvirt gets a DEVICE_DELETED event from qemu indicating that the guest is finished with the device on its end. The qemuDomainDetach*Device() functions should only contain code that ensures the requested operation is valid, and sends the command to qemu to initiate the unplug. * qemuDomainDetachExtensionDevice() is a function that applies to devices that plug into a PCI slot, *not* necessarily PCI controllers (which is what's being checked in the offending code). The proper way to check for this would be to see if the DeviceInfo for the controller device had a PCI address, not to check if the controller is a PCI controller (the code being removed was doing the latter). * According to commit 1d1e264f that added this code (and other support for hotplugging zPCI devices on s390), it's not necessary to explicitly detach the zPCI device when unplugging a PCI device. To quote: There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. and the evidence bears this out - all the other uses of qemuDomainDetachExtensionDevice() (except one, which I believe is also in error, and is being removed in a separate patch) are only to remove the zPCI extension device in cases where it was successfully added, but there was some other failure later in the hotplug process (so there was no regular PCI device to remove and trigger removal of the zPCI extension device). * PCI controllers are not hot pluggable, so this is dead code anyway. (The only controllers that can currently be hotplugged/unplugged are SCSI controllers). Signed-off-by: NLaine Stump <laine@laine.org> Reviewed-by: NJán Tomko <jtomko@redhat.com> Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
-
由 Michal Privoznik 提交于
After 7431b3eb libvirt requires "filter", "nat" and "mangle" tables to exist for both IPv4 and IPv6. This fact was missed in the news.xml and since we don't have any better place to advertise that let's update old news. This was refined in 686803a1 and since that is not released yet create a new entry documenting the refinement. Signed-off-by: NMichal Privoznik <mprivozn@redhat.com> Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
-
- 25 3月, 2019 6 次提交
-
-
test:///default由 Eric Blake 提交于
Had this been in place earlier, I would have avoided the bugs in commit 0baf6945 and 55c2ab3e. Writing the test required me to extend the power of virsh - creating enough snapshots to cause fanout requires enough input in a single session that adding comments and markers makes it easier to check that output is correct. It's still a bit odd that with test:///default, reverting to a snapshot changes the domain from running to paused (possibly a bug in how the test driver copied from the qemu driver) - but the important part is that the test is reproducible, and any future tweaks we make to snapshot code have less chance of breaking successful command sequences. Signed-off-by: NEric Blake <eblake@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Eric Blake 提交于
Since test:///default resets state on every connection, writing a test that covers a sequence of commands must be done from a single session. But if the test wants to exercise particular failure modes as well as successes, it can be nice to leave witnesses in the stderr stream immediately before and after the spot where the expected error should be, to ensure the rest of the script is not causing errors. Do this by adding an --err option. Signed-off-by: NEric Blake <eblake@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Eric Blake 提交于
As the previous commit mentioned, argv mode (such as when you feed virsh via stdin with <<\EOF instead of via a single shell argument) didn't permit comments. Do this by treating any command name token that starts with # as a comment which silently eats all remaining arguments to the next newline or semicolon. Note that batch mode recognizes unquoted # at the start of any word as a command as part of the tokenizer, while this patch only treats # at the start of the command word as a comment (any other # remaining by the time vshCommandParse() is processing things was already quoted during the tokenzier, and as such was probably intended as the actual argument to the command word earlier in the line). Now I can do something like: $ virsh -c test:///default <<EOF # setup snapshot-create-as test s1 snapshot-create-as test s2 # check snapshot-list test --name EOF Signed-off-by: NEric Blake <eblake@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Eric Blake 提交于
Continuing from what I did in commit 4817dec0, now I want to write a sequence that is self-documenting. So I need comments :) Now I can do something like: $ virsh -c test:///default ' # setup snapshot-create-as test s1 snapshot-create-as test s2 # check snapshot-list test --name ' Note that this does NOT accept comments in argv mode, another patch will tackle that. (If I'm not careful, I might turn virsh into a full-fledged 'sh' replacement? Here's hoping I don't go that far...) Signed-off-by: NEric Blake <eblake@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Eric Blake 提交于
Commit 55c2ab3e accidentally introduced an infinite loop while checking whether a redefined snapshot would cause an infinite loop in chasing its parents back to a root. Alas, 'make check' did not catch it, so my next patch will be a testsuite improvement that would have hung and prevented the bug from being checked in to begin with. Signed-off-by: NEric Blake <eblake@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Daniel Henrique Barboza 提交于
In a case where we want to hotplug the following disk: <disk type='file' device='disk'> (...) <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> In a QEMU guest that has a single OS disk, as follows: <disk type='file' device='disk'> (...) <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> What happens is that the existing guest disk will receive the ID 'scsi0-0-0-0' due to how Libvirt calculate the alias based on the address in qemu_alias.c, qemuAssignDeviceDiskAlias. When hotplugging a disk that happens to have the same address, Libvirt will calculate the same ID to it and attempt to device_add. QEMU will refuse it: $ virsh attach-device ub1810 hp-disk-dup.xml error: Failed to attach device from hp-disk-dup.xml error: internal error: unable to execute QEMU command 'device_add': Duplicate ID 'scsi0-0-0-0' for device And Libvirt follows it up with a cleanup code in qemuDomainAttachDiskGeneric that ends up removing what supposedly is a faulty hotplugged disk but, in this case, ends up being the original guest disk. This patch adds an address verification for all attached devices, avoid calling the driver attach() function using a device with duplicated address. The change is done in virDomainDefCompatibleDevice when @action is equal to VIR_DOMAIN_DEVICE_ACTION_ATTACH. The affected callers are: - qemuDomainAttachDeviceLiveAndConfig, both LIVE and CONFIG cases; - lxcDomainAttachDeviceFlags, both LIVE and CONFIG. The check is done using the virDomainDefHasDeviceAddress, a generic function that can check address duplicates for all supported device types, not limiting just to DeviceDisk type. After this patch, this is the result of the previous attach-device call: $ ./run tools/virsh attach-device ub1810 hp-disk-dup.xml error: Failed to attach device from hp-disk-dup.xml error: Requested operation is not valid: Domain already contains a device with the same address Reported-by: NSrikanth Aithal <bssrikanth@in.ibm.com> Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
-
- 22 3月, 2019 25 次提交
-
-
由 Cole Robinson 提交于
Base macro to unify the actual testCompareXMLToArgv test calls Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
* ARG_CAPS_ARCH must be specified with ARG_CAPS_VER * ARG_QEMU_CAPS shouldn't be specified with ARG_CAPS_* Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Move DO_CAPS_TEST* qemuCaps init and all the associated setup into testInfoSetArgs, adding ARG_CAPS_ARCH and ARG_CAPS_VER options and using those to build the capsfile path locally Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Make it an actual path and not a string prefix Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
These blocks are only triggered when updating from a libvirt version less than 0.9.4, which was released in August 2011. I think it's been long enough that we can say this upgrade path is unsupported without an intermediate step. Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Cole Robinson 提交于
Since Fedora 28 (our minimum supported build), ldconfig is called automatically for us: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets These changes appear to be implemented for RHEL > 7 as well, so only run ldconfig on RHEL7 Reviewed-by: NAndrea Bolognani <abologna@redhat.com> Signed-off-by: NCole Robinson <crobinso@redhat.com>
-
由 Andrea Bolognani 提交于
We have tests for simple guests with graphics for basically all other architectures, so it makes sense to include s390x too. The input file was generated by running $ virt-install \ --name guest --os-variant fedora29 \ --vcpus 4 --memory 4096 --disk size=5 \ --graphics vnc \ --print-xml followed by minor tweaks, using a version of virt-manager that includes commit 7b9de27a990f. Signed-off-by: NAndrea Bolognani <abologna@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Andrea Bolognani 提交于
As of commit db6c7070e25a, virt-manager will default to using virtio-blk rather than virtio-scsi for aarch64/virt guests, bringing them in line with other architectures. Update our test case to reflect this change. Signed-off-by: NAndrea Bolognani <abologna@redhat.com> Acked-by: NMichal Privoznik <mprivozn@redhat.com>
-
由 Michal Privoznik 提交于
This functions tries to add a domain moment (love the name!) onto a list of domain moments. Firstly, it checks if another moment with the same name already exists. Then, it creates an empty moment (without initializing its definition) and tries to add the moment onto the list dereferencing moment definition in that process. If it succeeds (which it never can), only after that it sets moment->def. Signed-off-by: NMichal Privoznik <mprivozn@redhat.com> Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
-
由 Nikolay Shirokovskiy 提交于
Commit 3bd4ed46 introduced this element as required which breaks backcompat for test driver. Let's make the element optional. Reviewed-by: NErik Skultety <eskultet@redhat.com> Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
-
由 Nikolay Shirokovskiy 提交于
Commit d1a7c08e changed filter instantiation code to ignore MAC and IP variables explicitly specified for filter binding. It just replaces explicit values with values associated with the binding. Before the commit virNWFilterCreateVarsFrom was used so that explicit value take precedence. Let's bring old behavior back. This is useful. For example if domain has two interfaces it makes sense to list both mac adresses in MAC var of every interface filterref. So that if guest make a bond of these interfaces and start sending frames with one of the mac adresses from both interfaces we can pass outgress traffic from both interfaces too. Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com> Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
-
由 Eric Blake 提交于
Now that the generic moment code does pretty much everything that both snapshots and checkpoints will need, it's time to replace the now-duplicate code in virdomainsnapshotobjlist.c with simpler calls into the generic code. I considered using sub-classing (a 'virDomainMomentObjList parent;' member, but that requires making the opaque type visible in headers; so for now, I stuck with a container instead (a 'virDomainMomentObjListPtr base;' member). Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
The new code here very heavily resembles the code in virDomainSnapshotObjList. There are still a couple of spots that are tied a little too heavily to snapshots (the free function lacks a polymorphic cleanup until we refactor code to use virObject; and an upcoming patch will add internal VIR_DOMAIN_MOMENT_LIST flags to replace the snapshot flag bits), but in general this is fairly close to the state needed to add checkpoint lists. Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 Eric Blake 提交于
Now that we have made virDomainMomentObj sufficiently generic to support both snapshots and checkpoints, it is time to rename the file that it lives in. The split between a generic object and a list of the generic objects doesn't buy us as much, so it will be easier to stick all the moment list code in one file, with more code moving in the next patch. The changes during the move are fairly minor, although it is worth pointing out that the log/error messages for the new file report that they are from "domain", since the file will eventually be shared by both "domain snapshot" and "domain checkpoint". Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
-
由 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>
-