Re: Support cloning of VMs - part 2

On Wed, Jul 09, 2025 at 01:42:20PM +0100, Mark Cave-Ayland wrote: > On 09/07/2025 12:10, Daniel P. Berrangé wrote: > >> On Wed, Jul 09, 2025 at 11:27:45AM +0100, Daniel P. Berrangé via Devel wrote: >>> On Mon, Jul 07, 2025 at 02:19:10PM +0100, Mark Cave-Ayland wrote: >>>> Hi all, >>>> >>>> I'm currently looking at how libvirt can be used to clone a saved VM, and >>>> have been focusing on the previous thread on this topic at https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.libvirt.org_ar... . >>>> >>>> My understanding from reading the thread is that the best way to approach >>>> this is 1) add uuid properties to the relevant objects/devices in QEMU where >>>> the UUID is user-visible, and 2) update libvirt to handle these properties >>>> via XML. >>>> >>>> Does this sound correct, or is there any more recent thinking about the best >>>> approach to take? >>> >>> Yes, that's still a reasonable way to approach this, unless we uncover >>> new info when you try implementing it. >> >> Well it didn't take me long to uncover new info :-( >> >> Looking at the QEMU codebase for where 'qemu_uuid' is used reveals rather >> more than anticipated >> >> git grep -E '\bqemu_uuid(_set)?\b' >> hw/core/machine-qmp-cmds.c: info->UUID = qemu_uuid_unparse_strdup(&qemu_uuid); >> hw/i386/vmport.c: uint32_t *uuid_parts = (uint32_t *)(qemu_uuid.data); >> hw/nvram/fw_cfg.c: fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16); >> hw/ppc/pnv.c: buf = qemu_uuid_unparse_strdup(&qemu_uuid); >> hw/ppc/pnv.c: if (qemu_uuid_set) { >> hw/ppc/spapr.c: buf = qemu_uuid_unparse_strdup(&qemu_uuid); >> hw/ppc/spapr.c: if (qemu_uuid_set) { >> hw/ppc/spapr_rtas.c: ret = sysparm_st(buffer, length, (unsigned char *)&qemu_uuid, >> hw/ppc/spapr_rtas.c: (qemu_uuid_set ? 16 : 0)); >> hw/smbios/smbios.c: if (qemu_uuid_set) { >> hw/smbios/smbios.c: smbios_encode_uuid(&t->uuid, &qemu_uuid); >> hw/smbios/smbios.c: if (qemu_uuid_parse(val, &qemu_uuid) != 0) { >> hw/smbios/smbios.c: qemu_uuid_set = true; >> hw/smbios/smbios_legacy.c: if (qemu_uuid_set) { >> hw/smbios/smbios_legacy.c: &qemu_uuid, 16); >> include/hw/firmware/smbios.h: /* uuid is in qemu_uuid */ >> include/sysemu/sysemu.h:extern QemuUUID qemu_uuid; >> include/sysemu/sysemu.h:extern bool qemu_uuid_set; >> migration/multifd.c: memcpy(msg.uuid, &qemu_uuid.data, sizeof(msg.uuid)); >> migration/multifd.c: if (memcmp(msg.uuid, &qemu_uuid, sizeof(qemu_uuid))) { >> migration/multifd.c: char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid); >> migration/savevm.c: state->uuid = qemu_uuid; >> migration/savevm.c: return qemu_uuid_set && migrate_validate_uuid(); >> migration/savevm.c: if (!qemu_uuid_set) { >> migration/savevm.c: if (!qemu_uuid_is_equal(&state->uuid, &qemu_uuid)) { >> migration/savevm.c: qemu_uuid_unparse(&qemu_uuid, uuid_dst); >> system/globals.c:/* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the >> system/globals.c:QemuUUID qemu_uuid; >> system/globals.c:bool qemu_uuid_set; >> system/vl.c: if (qemu_uuid_parse(optarg, &qemu_uuid) < 0) { >> system/vl.c: qemu_uuid_set = true; >> target/s390x/kvm/kvm.c: memcpy(sysib.vm[0].uuid, &qemu_uuid, sizeof(sysib.vm[0].uuid)); >> target/s390x/tcg/misc_helper.c: memcpy(sysib.sysib_322.vm[0].uuid, &qemu_uuid, >> ui/dbus.c: g_autofree char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid); >> ui/spice-core.c: spice_server_set_uuid(spice_server, (unsigned char *)&qemu_uuid); >> >> Only a small subset of those places have a directly exposed property that >> could override the default they obtain from the -uuid arg. >> >> The savevm code in particular is a sticking point as we need to use >> the migration code as the mechanism for cloning. >> >> So if we want to enable cloning, without the guest visible UUIDs changing, >> and with the savevm checks passing, we're pretty much stuck with having to >> set -uuid to a *different* value than the <uuid></uuid> in the XML :-( >> >> This probably takes us back to needing to have another element at the >> top level in the XML. >> >> <uuid>....</uuid> >> <hwuuid>...</hwuuid> >> >> where <uuid> is mandatory as today, and <hwuuid> can optionally be used >> to override what is exposed to the guest, defaulting to <uuid> if >> omitted. > > Thanks for the update, I'll take a look at implementing something along > these lines. Would it be slightly easier to add the guest-visible UUID as an > attribute to the existing uuid element e.g. > > <uuid guest-uuid="....">....</uuid> > > since then it should already be accessible at the places where we already > have an existing reference to the uuid element? We already have the 'genid' UUID as a separate element, so I figure we just carry on that practice with 'hwuuid'.