On Thu, May 14, 2026 at 04:58:18PM +0100, Daniel P. Berrangé wrote:
On Thu, May 14, 2026 at 05:51:25PM +0200, Martin Kletzander via Devel wrote:From: Martin Kletzander <[email protected]>The difference is that the usual UUID is supposed to be unique per host and instanceUuid should be unique across the whole cluster. One could think of them as HUID and CUID (as the first "U" does apparently mean something else in the Broadcom world). That _would_ be fine for our scenario. However, that piece of information turns out to be false as well and the UUID we were using (`config.uuid`, or in VMX the `uuid.bios`) can be the same in two machines on the same host. Fortunately the `FindByUuid()` function can also search for VMs based on their `instanceUuid`, dictated by the so far omitted third parameter. Unfortunately that parameter is not parsed (or at least properly) before vSphere API 4.0 (the documentation says 2.0, but we are not using that namespace and 4.0 is the lowest we can target), which we are not specifying in the server returns a 500 HTTP error if we use the `instanceUuid` parameter. So this patch adds the `SOAPAction: urn:vim25/4.0` header to the cURL requests which makes that `FindByUuid()` function work even with the `instanceUuid` set, but without any extra labor. After that this patch also changes all UUIDs to be parsed from the `config.instanceUuid` (or `vc.uuid` in the VMX, but there's a fallback to the old `uuid.bios`) and adjusts tests accordingly. To give users (and management applications) the possibility to revert back to the previous (legacy) behaviour a new URI query parameter is introduced, called `legacy_uuid` which, if set to `1`, still keeps the code working as it did before this patch. Last, but not least it changes the parameter to aforementioned function to be true (unless the legacy behaviour is requested, of course) and henceforth all searching ought to be done with the more unique ID. Resolves: https://redhat.atlassian.net/browse/RHEL-174300 Signed-off-by: Martin Kletzander <[email protected]> --- docs/drvesx.rst | 9 ++++++ src/esx/esx_driver.c | 40 +++++++++++++++--------- src/esx/esx_util.c | 11 +++++++ src/esx/esx_util.h | 1 + src/esx/esx_vi.c | 33 ++++++++++++++++--- src/esx/esx_vi.h | 3 ++ src/vmx/vmx.c | 10 ++++-- tests/vmx2xmldata/esx-in-the-wild-10.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-11.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-12.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-13.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-14.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-15.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-16.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-17.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-5.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-6.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-7.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-8.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-9.xml | 2 +- 20 files changed, 99 insertions(+), 34 deletions(-)diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 010c62b8e880..9e4746b4fcd4 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c@@ -4960,9 +4968,13 @@ esxDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) esxPrivate *priv = domain->conn->privateData; esxVI_ManagedObjectReference *managedObjectReference = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; + esxVI_Boolean instanceUuid = esxVI_Boolean_True; virCheckFlags(0, -1); + if (priv->primary->legacy_uuid) + instanceUuid = esxVI_Boolean_Undefined; +This code handles legacy_uuid param...if (esxVI_EnsureSession(priv->primary) < 0) return -1; @@ -4970,7 +4982,7 @@ esxDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) if (esxVI_FindByUuid(priv->primary, priv->primary->datacenter->_reference, uuid_string, esxVI_Boolean_True, - esxVI_Boolean_Undefined, + instanceUuid, &managedObjectReference) < 0) { return -1; }diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index db006ed16f40..f6e9c2c01e11 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c* 100 (Continue) from the server before continuing the POST operation. @@ -805,6 +816,7 @@ ESX_VI__TEMPLATE__FREE(Context, esxVI_SelectionSpec_Free(&item->selectSet_computeResourceToHost); esxVI_SelectionSpec_Free(&item->selectSet_computeResourceToParentToParent); esxVI_SelectionSpec_Free(&item->selectSet_datacenterToNetwork); +// g_free(item->uuid_key);left over debug ?
Yeah, I had it allocated before, thanks for spotting that.
}) int @@ -933,7 +945,8 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, if (ctx->productLine == esxVI_ProductLine_VPX) ctx->hasSessionIsActive = true; - + ctx->legacy_uuid = parsedUri->legacy_uuid; + ctx->uuid_key = ctx->legacy_uuid ? "config.uuid" : "config.instanceUuid";...this handles legacy_uuid...diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 084b4154427f..a2422c37355c 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1492,9 +1492,13 @@ virVMXParseConfig(virVMXContext *ctx, def->scsiBusMaxUnit = SCSI_SUPER_WIDE_BUS_MAX_CONT_UNIT; } - /* vmx:uuid.bios -> def:uuid */ - /* FIXME: Need to handle 'uuid.action = "create"' */ - if (virVMXGetConfigUUID(conf, "uuid.bios", def->uuid, true) < 0) + /* vmx:vc.uuid (fallback to uuid.bios) -> def:uuid */ + if (virVMXGetConfigUUID(conf, "vc.uuid", def->uuid, true) < 0) + goto cleanup; + + /* FIXME: Need to handle 'uuid.action = "create"' ? */ + if (!virUUIDIsValid(def->uuid) && + virVMXGetConfigUUID(conf, "uuid.bios", def->uuid, true) < 0) goto cleanup;This code does not handle legacy_uuid ? Should it ?
So this code handles VMX parsing for all the various users of that. For everyone other than the ESX driver I don't really see a reason to handle that. But now I see the inconsistency that arises from this fallback action, which should not happen with legacy_uuid=1.
Also same questino as previous versino - we should probably be populating the <hwuuid> in libvirt with the BIOS uuid, if we're not using that for the primary UUID anymore.
I forgot about that, sorry. Even though I'm not sure whether that really is the same thing as we describe in the docs (is it visible in the guest?), but it makes sense and it can also be used from the callers to decide if they want the legacy behaviour (replace the domain UUID by the <hwuuid>) or not. I guess I was too much in a rush, I'll fix it in v3, thanks for the review.
/* vmx:displayName -> def:name */With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
signature.asc
Description: PGP signature
