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

On 19/04/13 16:37, Li Zhang wrote: > From: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; > > This patch is to add command line builder and parser > for NVRAM device, and add test cases. > > Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; > --- > 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.