[fedora-virt] [fedora-specific libvirt PATCH] qemu: replace deprecated fedora-13 machine type with pc-0.13

[Eric Blake]

On 12/08/2011 03:08 PM, Laine Stump wrote:
> (I'm thinking of appying this patch to libvirt in the F15 and F16
> branches, as well as Rawhide. The intent is to prepare everyone for
> qemu's removal of support for the "fedora-13" machine type, which will
> happen in F17 (already done in Rawhide)).
> 
> This addresses https://bugzilla.redhat.com/show_bug.cgi?id=754772 .
> It should only be applied to Fedora builds of libvirt (it seems
> appropriate to apply it to all versions currently in support).
> 
> ---
>  src/conf/domain_conf.c                             |   52 ++++++++++++++++++-
>  .../qemuxml2argv-encrypted-disk.args               |    2 +-
>  .../qemuxml2argv-encrypted-disk.xml                |    2 +-

The 4 line change to the testsuite is probably worth proposing upstream
(upstream should try to favor upstream qemu names, rather than a
Fedora-specific name), but I agree with your decision to make the rest
of this patch Fedora-specific, and hope that by F18 we will have
automatically phased out enough of the offenders to not have to carry
the patch around any longer.

> +++ b/src/conf/domain_conf.c
> @@ -8061,7 +8061,25 @@ virDomainDefPtr virDomainDefParseString(virCapsPtr caps,
>                                          unsigned int expectedVirtTypes,
>                                          unsigned int flags)
>  {
> -    return virDomainDefParse(xmlStr, NULL, caps, expectedVirtTypes, flags);
> +    virDomainDefPtr def
> +        = virDomainDefParse(xmlStr, NULL, caps, expectedVirtTypes, flags);
> +
> +    /* Fedora-specific HACK - treat fedora-13 and pc-0.13 as equivalent.
> +     * This handles the case of domains that had been saved to an image file
> +     * prior to upgrade (save or snapshot), then restarted/reverted.
> +     */
> +    if (def && STREQ_NULLABLE(def->os.machine, "fedora-13")) {

We want the hack to only apply to qemu guests.  At first, I wondered if
maybe we should move this into files in src/qemu/qemu_*.c; but on
thinking about it more, that would make the problem get bigger, as there
are multiple code paths that call this function.  For the purposes of a
hack that we don't plan to support in F18, I can live with doing it
here.  But doing it here means we have to worry about this code being
shared by other drivers.  Therefore, I think you need one additional
piece of logic gating this hack:

 if (def->virtType == VIR_DOMAIN_VIRT_QEMU ||
     def->virtType == VIR_DOMAIN_VIRT_KQEMU ||
     def->virtType == VIR_DOMAIN_VIRT_KVM)

> @@ -11343,8 +11361,20 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>      virBufferAddLit(buf, "    <type");
>      if (def->os.arch)
>          virBufferAsprintf(buf, " arch='%s'", def->os.arch);
> -    if (def->os.machine)
> -        virBufferAsprintf(buf, " machine='%s'", def->os.machine);
> +    if (def->os.machine) {
> +        /* Fedora-specific HACK - replace "fedora-13" with "pc-0.13".
> +         * This will catch XML being written to save/migration images
> +         * of domains that were running when libvirtd was restarted at
> +         * the time of upgrade.
> +         */
> +        if (STREQ_NULLABLE(def->os.machine, "fedora-13")) {
> +            virBufferAddLit(buf, " machine='pc-0.13'");
> +            VIR_WARN("substituting machine type 'fedora-13' with 'pc-0.13' "
> +                     "in domain %s", def->name);
> +        } else {
> +            virBufferAsprintf(buf, " machine='%s'", def->os.machine);

Same comment about gating this to just qemu.  Also, do we want to print
the VIR_WARN on every dumpxml, or do we want to modify def->os.machine
in-place so that the remaining uses of the def are already at pc-0.13?

> @@ -11779,6 +11809,22 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps,
>                                        VIR_DOMAIN_XML_INACTIVE)))
>          goto error;
>  
> +    /* Fedora-specific HACK - replace "fedora-13" with "pc-0.13".
> +     * This updates all config files at the first restart of libvirt

s/libvirt/libvirtd/

> +     * after upgrade.
> +     */
> +    if (STREQ_NULLABLE(def->os.machine, "fedora-13")) {

Again, this should be gated to just qemu.

Except for the missing gating, it looks reasonable; I'll take some time
to test a domain snapshot taken before the patch and loaded after and
report back with that (but at first glance, it should work).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
Url : http://lists.fedoraproject.org/pipermail/virt/attachments/20111208/909ba401/attachment.bin