On Fri, Jan 17, 2025 at 01:11:54PM +0000, Felipe Franciosi wrote: > > > > On 17 Jan 2025, at 12:47, Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > > > > On Mon, Jan 13, 2025 at 12:13:43PM +0000, Felipe Franciosi wrote: > >> > >> > >>> On 10 Jan 2025, at 16:03, Felipe Franciosi <felipe@xxxxxxxxxxx> wrote: > >>> > >>>> On 10 Jan 2025, at 15:02, Daniel P. Berrangé <berrange@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: > >>>>>>>> > >>>>>>>> On Fri, Jan 10, 2025 at 12:09:44PM +0000, Felipe Franciosi wrote: > >>>>>>>>> > >>>>>>>>> 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! > >> > >> I did some more digging and found this: > >> > >> ------------8<------------ > >> commit 7998465005e2ebf26f6e65f5bdb886487374bb18 > >> Author: Daniel P. Berrangé <berrange@xxxxxxxxxx> > >> Date: Wed May 4 11:40:59 2011 +0100 > >> > >> Add field to virDomainObjPtr to track "tainting" > >> ------------8<------------ > >> > >> Seems like the idea of that taint parsing in virDomainObjParseXML() is to do > >> with <taint> tags added by virDomainObjFormat(). That answers one of my > >> questions (the one to do with where the tags come from). > >> > >> But I'm still unclear on how you'd prefer I raise this patch. Here are the > >> options I have in mind: > >> > >> 1. Add a <taint> tag while parsing in virSysinfoSystemParseXML() > >> (I don't really like this as no other "parse" method modifies the XML.) > >> > >> 2. Add a check somewhere later, e.g., in virDomainDefValidate() > >> (This feels better, but virDomainDefValidate() doesn't take the > >> virDomainObj, only the virDomainDef, so that may need to change.) > >> > >> 3. Taint the object in virDomainDefCheckABIStabilityFlags() > >> (This has the same problem as the option above, so I prefer the former.) > >> > >> 4. Check it explicitly in virDomainObjParseXML() > >> (That doesn't require passing the virDomainObj to the post-parsing methods, > >> but it just feels wrong as there are post-parsing methods.) > >> > >> Let me know what you think. > > > > Tainting is something we only apply to running VMs, so its hooked into > > the startup code path rather than XML parsing or validation time. > > > > So generally you'd want to add to "qemuDomainObjCheckTaint" or one of > > the helpers it calls. > > Yes! I had worked that out this week when I fiddle with it some more. I will > send a patchset for this soon. > > >>>>> 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? > >> > >> I still have the question above. > > > > Hmm, right, of course, SMBIOS can't change at runtime and even if we > > did change it, we can't expect the guest OS to update its view. > > > > Lets just forget the whole thing about providing the libvirt UUID > > elsewhere. If and when someone claims later we can decide whether > > to worry about it, or show them a different way to get to the same > > end goal. > > Upon reviewing some more code, my understanding is that the "-uuid" argument, > in QEMU, is used to define parts of the virtual hardware. However, in libvirt, > the <uuid> tag is used both to set the internal identifier for the VM and also > for defining the "-uuid" argument. I think that can be improved by: > > 1. Adding more options to QEMU (and corresponding libvirt code) which allow > fine-grained control of what UUIDs, in general, are used for different parts > of the virtual hardware. The QEMU -uuid therefore becomes merely a default. Yes, if a device needs to have a UUID, it ought to be controllable, even if it defaults to the overall machine UUID. > 2. (probably optional of "1" above is complete) Breaking the relationship of > libvirt's <uuid> and what it uses internally as a VM identifier OR the > relationship of libvirt's <uuid> (which can remain an identifier) and what is > passed to QEMU's -uuid. Don't think we need this, if we can control UUID on each individual device that wants a UUID. > 3. Proposing an official device type akin to Generation ID which hypervisors can > use to tell guests what is their hypervisor unique identifier. Based on your > response above, I think we both agree leveraging SMBIOS is not adequate > (although everyone does that... including Nutanix's Guest Tools). Yep, but not a priority IMHO. > > This can be done separately, though. My immediate concern is with VM fast-clone > (the whole save-clone-restore idea) whilst maintaining the same virtual hardware > (and those VMs do not have guest tools in my current use case). Thoughts welcome! > > ps. Congratulations on the big release earlier this week! Technically no libvirt releases are "big" releases. Both the major & minor parts of the version number are just boring time counters with no semantics wrt what's in the release :-) We follow a strictly time based release cadence, on the 1st of each month, except in Jan/Feb where we do a Jan 15th release instead of Jan 1st/Feb 1st to avoid new year holidays. We just bump the major version in Jan every year. QEMU does a similar bump major version for first release of each year. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|