Re: [PATCH 0/1] Support cloning saved VMs

On 10 Jan 2025, at 16:03, Felipe Franciosi <felipe(a)nutanix.com&gt; wrote: > On 10 Jan 2025, at 15:02, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > On Fri, Jan 10, 2025 at 01:46:53PM +0000, Felipe Franciosi wrote: >> >> >>> On 10 Jan 2025, at 12:38, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: >>> >>> On Fri, Jan 10, 2025 at 12:31:00PM +0000, Felipe Franciosi wrote: >>>> >>>> >>>>> On 10 Jan 2025, at 12:23, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: >>>>> >>>>> On Fri, Jan 10, 2025 at 12:09:44PM +0000, Felipe Franciosi wrote: >>>>>> >>>>>> >>>>>>> On 10 Jan 2025, at 11:00, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: >>>>>>> >>>>>>> On Fri, Jan 10, 2025 at 10:49:20AM +0000, Felipe Franciosi wrote: >>>>>>>> >>>>>>>> >>>>>>>>> On 10 Jan 2025, at 08:33, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: >>>>>>>>> >>>>>>>>> On Thu, Jan 09, 2025 at 07:27:16PM +0000, Felipe Franciosi wrote: >>>>>>>>>> Hello! >>>>>>>>>> >>>>>>>>>> I have a use case which I'm struggling to support with libvirt: >>>>>>>>>> saving a VM to a file, cloning it (which renames the VM), and restoring it. >>>>>>>>>> >>>>>>>>>> My search revealed a number of tutorials for using virt-clone [1], but that >>>>>>>>>> doesn't seem to cover VMs which are _saved_ (only running or paused). >>>>>>>>> >>>>>>>>> Saved in what way ? Managed save ? >>>>>>>> >>>>>>>> Thanks for the prompt reply! >>>>>>>> >>>>>>>> I'm saving with virDomainSave(). My understanding is that this is not managed. >>>>>>> >>>>>>> Functionally it is the same as managed save, just the that file path >>>>>>> is specified by the client, rather than by libvirt. >>>>>> >>>>>> Got it, thanks. >>>>>> >>>>>>>>>> [1] https://github.com/virt-manager/virt-manager/blob/main/virtinst/virtclone.py >>>>>>>>>> >>>>>>>>>> In a nutshell, I want to power on a VM and do some setup, then save its full >>>>>>>>>> state to disk (e.g., with virsh save). Finally I want to modify the XML to: >>>>>>>>>> - rename the VM >>>>>>>>>> - change which bridge its NICs are on (while maintaining mac addresses) >>>>>>>>>> - change the disk image to a copy (done while the VM is saved) >>>>>>>>>> >>>>>>>>>> But the restore operation fails because of a target domain name check >>>>>>>>>> implemented in virDomainDefCheckABIStabilityFlags(). I've debated how to best >>>>>>>>>> address this and I'm looking for your views. >>>>>>>>> >>>>>>>>> If you're cloning a VM, it needs both a new UUID and name, so I'm surprised >>>>>>>>> the ABI stability check hasn't already blocked you on the UUID change before >>>>>>>>> getting to the name change check. >>>>>>>> >>>>>>>> I definitely didn't change the UUID. In fact, I want it to be the same (at >>>>>>>> least in the SMBIOS tables) because the guest OS is not going to expect that >>>>>>>> value to change without a power cycle/reset. The ABI Check actually ensures the >>>>>>>> SMBIOS values do not change during restore. >>>>>>>> >>>>>>>> https://gitlab.com/libvirt/libvirt/-/blob/caa10431cdd1aa476637ff721f1947c... >>>>>>>> >>>>>>>> My understanding is that this passed because the other domain was not running >>>>>>>> (and the save was unmanaged, so libvirt is unaware of the saved VM). >>>>>>>> >>>>>>>> What I don't understand is why the UUID has to be unique (or, in fact, the same >>>>>>>> as the SMBIOS Type 1 UUID). Isn't this something just visible to the VM? For >>>>>>>> the clone use case, I surely don't want this to change. >>>>>>>> >>>>>>>> In other words, it's not clear to me why this check is needed: >>>>>>>> https://gitlab.com/libvirt/libvirt/-/blob/caa10431cdd1aa476637ff721f1947c... >>>>>>> >>>>>>> Libvirt has three unique identifiers for all VMs - UUID, name, and ID. The >>>>>>> latter is only for running VMs. >>>>>>> >>>>>>> UUID is the primary unique identifier that is used for pretty much every >>>>>>> lookup inside libvirt. Name is a secondary unique identifier largely just >>>>>>> for external lookups by humans, since UUIDs are not human friendly. >>>>>>> >>>>>>> Essentially every API call starts with virDomainObjListFindByUUID to convert >>>>>>> the public 'virDomainPtr' object into the internal 'virDomainObjPtr' struct >>>>>>> that holds the config & state. >>>>>> >>>>>> Ah-ha. Ok, this is really helpful, thanks again! >>>>>> >>>>>> My next question is why the SMBIOS Type 1 UUID tied to the Libvirt identifier? >>>>>> (I'm pointing again at L#12810 above.) >>>>>> >>>>>> That feels incorrect. My (new) understanding is that: >>>>>> - The SMBIOS Type 1 UUID is guest-visible >>>>>> - The Libvirt UUID is a host identifier >>>>>> >>>>>> What comes to mind is that maybe something like guest tools wants to be able to >>>>>> report back to a control plane what VM it is on based on this value. If that's >>>>>> the motivation, then isn't Generation ID a better field to rely on? >>>>> >>>>> Strictly speaking we don't have to tie them together, but in practice we >>>>> do, because it is pretty compelling to be correlate data between the host >>>>> OS and guest OS for apps. >>>> >>>> Right, so libvirt uses the XML <UUID> as a host unique identifier and also as >>>> QEMU's "-uuid" parameter (which, based on my understanding, is a default for >>>> any virtual hardware UUID stuff such as SMBIOS Type 1 UUID/Serial. And the >>>> reason for that is to allow guests to infer their hypervisor identifier. >>>> >>>>> >>>>>> My understanding is that SMBIOS identifiers cannot change at runtime. >>>>> >>>>> Correct. >>>> >>>> Ok, but then I'm confused: how is clone supposed to work? When I clone a saved >>>> VM, Libvirt requires that I change its <UUID> and it also requires that this >>>> <UUID> matches SMBIOS Type 1 UUID which, by definition, cannot change. >>>> >>>> What am I missing? >>> >>> Yep, the idea of cloning a running (well saved) VM on the same >>> host is effectively denied due to this policy. >>> >>> We have never claimed that cloning a running VM is supported, >>> and actively discouraged people from trying to do this. >>> >>> The only workaround would be if launched on a different host. Failing >>> that the only option would be for us to remove the requirement that >>> VM UUID matches SMBIOS UUID. >>> >>> Perhaps we could do the latter, but mark the VM as "tainted" to indicate >>> this undesirable config scenario. >> >> That works for me. Let me submit another patch along those lines for review. I'm having a look at this and it isn't entirely clear to me how to do it. I can see that virDomainObjParseXML() creates the virDomainObj *obj. Most of the interesting parsing then happens in virDomainDefParseXML() which (sensibly) takes the xmlXPathContextPtr and not the entire virDomainObj. That means, however, that I can't directly call virDomainObjTaint() from the relevant parsing bit in virSysinfoSystemParseXML() to taint the obj with my newly introduced VIR_DOMAIN_TAINT_UUID_MISMATCH. However, I can see that further down in virDomainObjParseXML() there's a virXPathNodeSet("./taint", ctxt, &taintNodes). I'm therefore guessing I don't have to call virDomainObjTaint() while parsing the XML. Instead, maybe I can just add "./taint" to the XML while parsing? I can't immediately find any precedence to that, though. Directions welcome!

>> Also it sounds like there isn't a strong reason for tying up SMBIOS UUID and VM >> UUID except the use case of the guest inferring its hypervisor identifier. >> Would it make sense to propose a new device type that can canonically be used >> for that purpose? Something like Generation ID, perhaps. I can see if someone >> from our side can work on that if you think it's a good idea. > > I'd suggest SMBIOS can already handle this. eg We could just document > something along the lines if > > If you make "System" (Type 1 table) "UUID" different from machine UUID, > then set "Base Board" (Type 2 table) "Asset Tag" to hold the machine UUID But that would mean the Asset Tag could change at VM runtime (well, at least while saved to disk). Wouldn't that be a problem?