Re: Support cloning of VMs - part 2

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?