[libvirt] [PATCH v5 2/2] qemu: Add command line builder and parser for NVRAM.

Li Zhang zhlcindy at gmail.com
Thu Apr 25 02:15:35 UTC 2013


On 2013年04月24日 20:41, Osier Yang wrote:
> On 24/04/13 20:17, Osier Yang wrote:
>> On 19/04/13 16:37, Li Zhang wrote:
>>> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>>
>>> This patch is to add command line builder and parser
>>> for NVRAM device, and add test cases.
>>>
>>> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>> ---
>>>   v5 -> v4:
>>>    * Fix one memory leakage suggested by Eric Blake
>>>    * Fix several code style suggested by Eric Blake
>>>
>>>   src/qemu/qemu_capabilities.c |  3 ++
>>>   src/qemu/qemu_capabilities.h |  2 +
>>>   src/qemu/qemu_command.c      | 88 
>>> ++++++++++++++++++++++++++++++++++++++++++--
>>>   tests/qemuargv2xmltest.c     |  2 +
>>>   tests/qemuxml2argvtest.c     |  1 +
>>>   tests/qemuxml2xmltest.c      |  2 +
>>>   6 files changed, 95 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_capabilities.c 
>>> b/src/qemu/qemu_capabilities.c
>>> index ef291c0..1d54477 100644
>>> --- a/src/qemu/qemu_capabilities.c
>>> +++ b/src/qemu/qemu_capabilities.c
>>> @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>>>                 "machine-usb-opt",
>>>                 "tpm-passthrough",
>>>                 "tpm-tis",
>>> +
>>> +              "nvram",  /* 140 */
>>>       );
>>>     struct _virQEMUCaps {
>>> @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags 
>>> virQEMUCapsObjectTypes[] = {
>>>       { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG },
>>>       { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM },
>>>       { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD },
>>> +    { "spapr-nvram", QEMU_CAPS_DEVICE_NVRAM },
>>>   };
>>>     static struct virQEMUCapsStringFlags 
>>> virQEMUCapsObjectPropsVirtioBlk[] = {
>>> diff --git a/src/qemu/qemu_capabilities.h 
>>> b/src/qemu/qemu_capabilities.h
>>> index 4e76799..85f47c4 100644
>>> --- a/src/qemu/qemu_capabilities.h
>>> +++ b/src/qemu/qemu_capabilities.h
>>> @@ -179,6 +179,8 @@ enum virQEMUCapsFlags {
>>>       QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */
>>>       QEMU_CAPS_DEVICE_TPM_TIS     = 139, /* -device tpm_tis */
>>>   +    QEMU_CAPS_DEVICE_NVRAM       = 140,  /*-global 
>>> spapr-nvram.reg=xxxx*/
>>
>> /* -global spapr-nvram.reg=xxxx */
>>
>>
>>> +
>>>       QEMU_CAPS_LAST,                   /* this must always be the 
>>> last item */
>>>   };
>>>   diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 009d42d..41b8d78 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -53,6 +53,10 @@
>>>     #define VIR_FROM_THIS VIR_FROM_QEMU
>>>   +#define VIO_ADDR_NET 0x1000ul
>>> +#define VIO_ADDR_SCSI 0x2000ul
>>> +#define VIO_ADDR_SERIAL 0x30000000ul
>>> +#define VIO_ADDR_NVRAM 0x3000ul
>>
>> This is much better.
>>
>>>     VIR_ENUM_DECL(virDomainDiskQEMUBus)
>>>   VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
>>> @@ -1148,7 +1152,7 @@ int 
>>> qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
>>>               STREQ(def->nets[i]->model, "spapr-vlan"))
>>>               def->nets[i]->info.type = 
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>>>           if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info,
>>> -                                      0x1000ul) < 0)
>>> +                                      VIO_ADDR_NET) < 0)
>>>               goto cleanup;
>>>       }
>>>   @@ -1163,7 +1167,7 @@ int 
>>> qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
>>>               def->controllers[i]->type == 
>>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
>>>               def->controllers[i]->info.type = 
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>>>           if (qemuAssignSpaprVIOAddress(def, 
>>> &def->controllers[i]->info,
>>> -                                      0x2000ul) < 0)
>>> +                                      VIO_ADDR_SCSI) < 0)
>>>               goto cleanup;
>>>       }
>>>   @@ -1173,7 +1177,16 @@ int 
>>> qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
>>>               STREQ(def->os.machine, "pseries"))
>>>               def->serials[i]->info.type = 
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>>>           if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info,
>>> -                                      0x30000000ul) < 0)
>>> +                                      VIO_ADDR_SERIAL) < 0)
>>> +            goto cleanup;
>>> +    }
>>> +
>>> +    if (def->nvram) {
>>> +        if (def->os.arch == VIR_ARCH_PPC64 &&
>>> +            STREQ(def->os.machine, "pseries"))
>>> +            def->nvram->info.type = 
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>>> +        if (qemuAssignSpaprVIOAddress(def, &def->nvram->info,
>>> +                                      VIO_ADDR_NVRAM) < 0)
>>>               goto cleanup;
>>>       }
>>>   @@ -3969,6 +3982,32 @@ error:
>>>       return NULL;
>>>   }
>>>   +static char *
>>> +qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev)
>>> +{
>>> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
>>> +
>>> +    if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO &&
>>> +        dev->info.addr.spaprvio.has_reg) {
>>> +        virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx",
>>> +                          dev->info.addr.spaprvio.reg);
>>> +    } else {
>>> +        virReportError(VIR_ERR_XML_ERROR,
>>> +                       "%s", _("NVRAM address only can be spaprvio 
>>> currently.\n"));
>>
>> "nvram address type must be 'spaprvio'"
>>
>> No need for the ".\n".
>>
>>> +        goto error;
>>> +    }
>>> +
>>> +    if (virBufferError(&buf)) {
>>> +        virReportOOMError();
>>> +        goto error;
>>> +    }
>>> +
>>> +    return virBufferContentAndReset(&buf);
>>> +
>>> +error:
>>> +    virBufferFreeAndReset(&buf);
>>> +    return NULL;
>>> +}
>>>     char *
>>>   qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
>>> @@ -7776,6 +7815,30 @@ qemuBuildCommandLine(virConnectPtr conn,
>>>               goto error;
>>>       }
>>>   +    if (def->nvram) {
>>> +        if (def->os.arch == VIR_ARCH_PPC64 &&
>>> +            STREQ(def->os.machine, "pseries")) {
>>> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) {
>>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> +                               "%s", _("NVRAM device is not 
>>> available "
>>
>> s/NVRAM/nvram/, to be consistent with the tag name in XML.
>>
>>> +                                " with this QEMU binary"));
>>
>> s/ with/with/
>>
>>> +                goto error;
>>> +            }
>>> +
>>> +            char *optstr;
>>> +            virCommandAddArg(cmd, "-global");
>>> +            optstr = qemuBuildNVRAMDevStr(def->nvram);
>>> +            if (!optstr)
>>> +                goto error;
>>> +            if (optstr)
>>> +                virCommandAddArg(cmd, optstr);
>>> +            VIR_FREE(optstr);
>>> +        } else {
>>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> +                          _("NVRAM device is only supported for 
>>> PPC64"));
>>
>> s/NVRAM/nvram/,
>>
>>> +            goto error;
>>> +        }
>>> +    }
>>>       if (snapshot)
>>>           virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, 
>>> NULL);
>>>   @@ -9884,6 +9947,25 @@ virDomainDefPtr 
>>> qemuParseCommandLine(virCapsPtr qemuCaps,
>>>                   goto error;
>>>               }
>>>   +        } else if (STREQ(arg, "-global") &&
>>> +                   STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) {
>>> +
>> useless blank line.
>>
>>> +            WANT_VALUE();
>>> +
>>> +            if (VIR_ALLOC(def->nvram) < 0)
>>> +                goto no_memory;
>>> +
>>> +            def->nvram->info.type = 
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>>> +            def->nvram->info.addr.spaprvio.has_reg = true;
>>> +
>>> +            val += strlen("spapr-nvram.reg=");
>>> +            if (virStrToLong_ull(val, NULL, 16,
>>> + &def->nvram->info.addr.spaprvio.reg) < 0) {
>>> +                virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +                               _("cannot parse nvram's address:"
>>> +                                 "'%s'"), val);
>>> +                goto error;
>>> +            }
>>>           } else if (STREQ(arg, "-S")) {
>>>               /* ignore, always added by libvirt */
>>>           } else {
>>> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
>>> index ee6c7a9..9f1bb24 100644
>>> --- a/tests/qemuargv2xmltest.c
>>> +++ b/tests/qemuargv2xmltest.c
>>> @@ -244,6 +244,8 @@ mymain(void)
>>>         DO_TEST("hyperv");
>>>   +    DO_TEST("pseries-nvram");
>>> +
>>>       DO_TEST_FULL("restore-v1", 0, "stdio");
>>>       DO_TEST_FULL("restore-v2", 0, "stdio");
>>>       DO_TEST_FULL("restore-v2", 0, "exec:cat");
>>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>>> index 4bf13f0..1c21a63 100644
>>> --- a/tests/qemuxml2argvtest.c
>>> +++ b/tests/qemuxml2argvtest.c
>>> @@ -907,6 +907,7 @@ mymain(void)
>>>               QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, 
>>> QEMU_CAPS_NODEFCONFIG);
>>>       DO_TEST_ERROR("pseries-vio-address-clash", QEMU_CAPS_DRIVE,
>>>               QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, 
>>> QEMU_CAPS_NODEFCONFIG);
>>> +    DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM);
>>>       DO_TEST("disk-ide-drive-split",
>>>               QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
>>>               QEMU_CAPS_IDE_CD);
>>> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>>> index 7434190..1d10bf2 100644
>>> --- a/tests/qemuxml2xmltest.c
>>> +++ b/tests/qemuxml2xmltest.c
>>> @@ -259,6 +259,8 @@ mymain(void)
>>>       DO_TEST("virtio-rng-random");
>>>       DO_TEST("virtio-rng-egd");
>>>   +    DO_TEST("pseries-nvram");
>>> +
>>>       /* These tests generate different XML */
>>>       DO_TEST_DIFFERENT("balloon-device-auto");
>>>       DO_TEST_DIFFERENT("channel-virtio-auto");
>>
>> ACK with the nits fixed.
>>
>
> I intended to push this with the attached diff squashed in. But it 
> fails with test
> files missed:

Ah, sorry. I forget to add these files to my patch.
I will resend it later. :)

>
>       .............................I/O warning : failed to load 
> external entity 
> "/home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml"
> !.......... 280
>       .                                        281 FAIL
> FAIL: qemuxml2argvtest
> TEST: qemuxml2xmltest
>       ........................................ 40
>       ........................................ 80
> ..................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: 
> failed to open: No such file or directory
> /home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml: 
> failed to open: No such file or directory
> !... 120
>       ...........                              131 FAIL
> FAIL: qemuxml2xmltest
> TEST: qemuxmlnstest
>       .......                                  7   OK
> PASS: qemuxmlnstest
> TEST: qemuargv2xmltest
>       ........................................ 40
> ...................................../home/osier/work/git/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args: 
> failed to open: No such file or directory




More information about the libvir-list mailing list