[libvirt] [PATCH v5 2/2] qemu: Add command line builder and parser for NVRAM.
Osier Yang
jyang at redhat.com
Wed Apr 24 12:41:33 UTC 2013
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:
.............................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
-------------- next part --------------
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 85f47c4..9c2bf57 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -179,7 +179,7 @@ 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*/
+ QEMU_CAPS_DEVICE_NVRAM = 140, /* -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 6b783e7..8f07639 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3946,8 +3946,8 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev)
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"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("nvram address type must be spaprvio"));
goto error;
}
@@ -7816,9 +7816,9 @@ qemuBuildCommandLine(virConnectPtr conn,
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 "
- " with this QEMU binary"));
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("nvram device is not supported by "
+ "this QEMU binary"));
goto error;
}
@@ -7832,7 +7832,7 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(optstr);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("NVRAM device is only supported for PPC64"));
+ _("nvram device is only supported for PPC64"));
goto error;
}
}
@@ -9946,7 +9946,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
} else if (STREQ(arg, "-global") &&
STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) {
-
WANT_VALUE();
if (VIR_ALLOC(def->nvram) < 0)
@@ -9959,8 +9958,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
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);
+ _("cannot parse nvram's address '%s'"), val);
goto error;
}
} else if (STREQ(arg, "-S")) {
More information about the libvir-list
mailing list