> 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.
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.
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).
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!
F.
