Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support

On 04/25/2018 08:41 AM, Daniel P. Berrangé wrote: > On Wed, Apr 25, 2018 at 02:36:52PM +0200, Peter Krempa wrote: >> On Wed, Apr 25, 2018 at 13:25:57 +0100, Daniel Berrange wrote: >>> On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: >>>> >>>> >>>> On 04/25/2018 04:46 AM, Peter Krempa wrote: >>>>> On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: >>>>>> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: >>>>>>> >>>>>>> Well, that depends. I did not read the docs for this thoroughly enough. >>>>>>> If it is okay for us to generate a new GUID upon every boot of a VM then >>>>>>> this will be for a rather simple implementation, since we have a very >>>>>>> limited set of situations when we are starting a new qemu process which >>>>>>> should NOT change the GUID and we will change it in all other scenarios. >>>>>> >>>>>> AFAIK, we *must* change GUID on every cold boot >>>>> >>>>> Good, that makes things rather simple. >>>>> >>>> >>>> This one is not really 100% clear to me. The "spec" is like 6 pages - >>>> it's a pretty quick read... >>>> >>>> http://go.microsoft.com/fwlink/?LinkId=260709 >>>> >>>> The last 2 pages describe "when" the GUID should change and specifically >>>> calling out cold boot is not one of those. What leads me to believe >>>> otherwise is the two boot scenarios and the unspecified VM config >>>> changes have this "undertone" that using the same GUID for those >>>> scenarios is fine/expected. >>> >>> Yeah the table at the very end is the key bit and it seems I was actually >>> wrong >>> >>> Scenario GenID changed >>> ----------------------------------------------------------------------- >>> Virtual machine is paused or resumed No >>> Virtual machine reboots No >>> Virtual machine host reboots No >>> Virtual machine starts executing a snapshot Yes >>> Virtual machine is recovered from backup Yes >>> Virtual machine is failed over in a disaster recovery env Yes >>> Virtual machine is live migrated No >>> Virtual machine is imported, copied, or cloned Yes >>> Virtual machine is failed over in a clustered environment No >>> Virtual machine's configuration changes Unspecified >>> >>> >>> My reading of "Virtual machine reboots" and "Virtual machine host reboots" >>> rows in particular is that we can *NOT* change GUID on every boot up >>> operation. The spec literally only wants it to be changed when there is >>> the possibility that the VM is potentially re-executing something that >>> has already been executed before. >>> >>> The transient VM feature is the real killer for libvirt - we have no >>> way of knowing when virDomainCreateXML launches a transient VM whether >>> that is starting up after a revert to backup/snapshot, or whether it >>> is a normal boot. Only the mgmt app like oVirt / OpenStack has enough >>> info to decide that. So we must allow the mgmt app to provide a GUID >>> in the XML document themselves, and then change it in places where we >>> know it is needed to change. Also allows virt-clone to adjust it too... >> >> Okay. So that means that we actually need to generate it in the parser, >> but we should also always report it back even for offline >> configurations. >> >> The only problem then will be what to do with the save/restore >> functionality, because that is really unknown to us since that API both >> includes the "Virtual machine is paused or resumed" and "Virtual machine >> starts executing a snapshot" scenario. > > I think it would fall under the "starts executing a snapshot" scenario > no matter what, because the spec doesn't say anything about whether the > original VM carried on running after the snapshot was taken. Just that > a snapshot was taken and this snapshot is then run. So the fact that > our save/restore can be view as a way to "pause" execution doesn't > matter - we've implemented "pause" by creating a snapshot and then > "resume" by running the snapshot. > So given all this - beyond not altering virDomainDefCopy to add a new flag and removing the ABI flag since it was only put there because of RevertToSnapshot @config copy difference, does just altering the genid via the START_GEN_VMID flag then cover things? Is there some other transition that needs that? IOW: Drop patches 2, 3 & 5... Merge patch 4 & 6 and alter 8/9 to not worry about abiflags. Do we print the GUID on inactive domains where we generate? I would think it wouldn't matter and the answer should be no. The print would only happen when active, but it's not that important.