On 09/07/2025 13:57, Daniel P. Berrangé wrote:
> 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'.
Thanks for the hints! I've been looking at the various drivers within
libvirt, and from what I can see not all virtualisation platforms will allow
a separate UUID to be provided in this way.
Would the best way forward be to add a new capability to represent this
ability, and then follow up with a QEMU implementation?
I think its probably overkill to add capability. Just define the XML
and wire up QEMU.
With regards,
Daniel
--
|: