[libvirt] [PATCH v2 0/4] Explicit boot device ordering
Currently, boot order can be specified per device class but there is no way to specify exact disk/NIC device to boot from. This patchset fixes that. There were two options suggested for how this should be modeled in domain XML: 1) <os> <boot target='net1'/> <boot target='net0'/> <boot target='hdc'/> <boot target='hdb'/> <os> Where target attributes would match /domain/devices/*/target@dev 2) Adding <boot order='n'/> elements into appropriate device elements. In addition to the two options Rich suggested a more compact variant of option 1. Per former discussion this patchset implements the second option. Version 2: - "qemu: Refactor qemuCapsParsePCIDeviceStrs using virCommand" dropped the rest was rebased on top of Eric's capabilities patches - added support for floppy devices which was forgotten in v1 Jiri Denemark (4): conf: Move boot parsing into a separate function Introduce per-device boot element qemu: Support per-device boot ordering tests: Add tests for per-device boot elements docs/formatcaps.html.in | 1 + docs/formatdomain.html.in | 41 +++++- docs/schemas/domain.rng | 20 +++- src/conf/capabilities.c | 3 +- src/conf/domain_conf.c | 148 ++++++++++++++------ src/conf/domain_conf.h | 2 + src/qemu/qemu_capabilities.c | 9 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 +++- src/qemu/qemu_command.h | 6 +- src/qemu/qemu_driver.c | 2 + src/qemu/qemu_hotplug.c | 8 +- .../qemuxml2argvdata/qemuxml2argv-boot-order.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 52 +++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 16 files changed, 265 insertions(+), 56 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml -- 1.7.4.rc1
--- src/conf/domain_conf.c | 99 ++++++++++++++++++++++++++++-------------------- 1 files changed, 58 insertions(+), 41 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2c54683..7c7a6fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4583,6 +4583,61 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, return retemu; } +static int +virDomainDefParseBootXML(xmlXPathContextPtr ctxt, + virDomainDefPtr def) +{ + xmlNodePtr *nodes = NULL; + int i, n; + char *bootstr; + int ret = -1; + + /* analysis of the boot devices */ + if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract boot device")); + goto cleanup; + } + + for (i = 0 ; i < n && i < VIR_DOMAIN_BOOT_LAST ; i++) { + int val; + char *dev = virXMLPropString(nodes[i], "dev"); + if (!dev) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing boot device")); + goto cleanup; + } + if ((val = virDomainBootTypeFromString(dev)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown boot device '%s'"), + dev); + VIR_FREE(dev); + goto cleanup; + } + VIR_FREE(dev); + def->os.bootDevs[def->os.nBootDevs++] = val; + } + if (def->os.nBootDevs == 0) { + def->os.nBootDevs = 1; + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; + } + + bootstr = virXPathString("string(./os/bootmenu[1]/@enable)", ctxt); + if (bootstr) { + if (STREQ(bootstr, "yes")) + def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_ENABLED; + else + def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED; + VIR_FREE(bootstr); + } + + ret = 0; + +cleanup: + VIR_FREE(nodes); + return ret; +} + static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, xmlDocPtr xml, xmlNodePtr root, @@ -4912,47 +4967,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.loader = virXPathString("string(./os/loader[1])", ctxt); } - if (STREQ(def->os.type, "hvm")) { - char *bootstr; - - /* analysis of the boot devices */ - if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract boot device")); - goto error; - } - for (i = 0 ; i < n && i < VIR_DOMAIN_BOOT_LAST ; i++) { - int val; - char *dev = virXMLPropString(nodes[i], "dev"); - if (!dev) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing boot device")); - goto error; - } - if ((val = virDomainBootTypeFromString(dev)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown boot device '%s'"), - dev); - VIR_FREE(dev); - goto error; - } - VIR_FREE(dev); - def->os.bootDevs[def->os.nBootDevs++] = val; - } - if (def->os.nBootDevs == 0) { - def->os.nBootDevs = 1; - def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; - } - VIR_FREE(nodes); - - bootstr = virXPathString("string(./os/bootmenu[1]/@enable)", ctxt); - if (bootstr) { - if (STREQ(bootstr, "yes")) - def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_ENABLED; - else - def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED; - VIR_FREE(bootstr); - } + if (STREQ(def->os.type, "hvm") && + virDomainDefParseBootXML(ctxt, def) < 0) { + goto error; } def->emulator = virXPathString("string(./devices/emulator[1])", ctxt); -- 1.7.4.rc1
On 01/14/2011 10:04 AM, Jiri Denemark wrote:
--- src/conf/domain_conf.c | 99 ++++++++++++++++++++++++++++-------------------- 1 files changed, 58 insertions(+), 41 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
ACK; simple refactoring. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Jan 14, 2011 at 06:04:43PM +0100, Jiri Denemark wrote:
--- src/conf/domain_conf.c | 99 ++++++++++++++++++++++++++++-------------------- 1 files changed, 58 insertions(+), 41 deletions(-)
ACK Daniel
Currently, boot order can be specified per device class but there is no way to specify exact disk/NIC device to boot from. This patch adds <boot order='N'/> element which can be used inside <disk/> and <interface/>. This is incompatible with the older os/boot element. Since not all hypervisors support per-device boot specification, new deviceboot flag is included in capabilities XML for hypervisors which understand the new boot element. Presence of the flag allows (but doesn't require) users to use the new style boot order specification. --- docs/formatcaps.html.in | 1 + docs/formatdomain.html.in | 41 ++++++++++++++++++++++++++++++++++- docs/schemas/domain.rng | 20 +++++++++++++++- src/conf/capabilities.c | 3 +- src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 + 6 files changed, 112 insertions(+), 6 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index dcbf35a..a4297ce 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -55,6 +55,7 @@ BIOS you will see</p> </arch> <features> <cpuselection/> + <deviceboot/> </features> </guest></span> ... diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dad268d..3afea8d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -103,8 +103,11 @@ <dd>The <code>dev</code> attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device to consider. The <code>boot</code> element can be repeated multiple - times to setup a priority list of boot devices to try in turn. - <span class="since">Since 0.1.3</span> + times to setup a priority list of boot devices to try in turn. The + <code>boot</code> element cannot be used if per-device boot elements + are used (see <a href="#elementsDisks">disks</a> and + <a href="#elementsNICS">network interfaces</a> sections below. + <span class="since">Since 0.1.3, per-device boot since 0.8.8</span> </dd> <dt><code>bootmenu</code></dt> <dd> Whether or not to enable an interactive boot menu prompt on guest @@ -625,6 +628,7 @@ <driver name="tap" type="aio" cache="default"/> <source file='/var/lib/xen/images/fv0'/> <target dev='hda' bus='ide'/> + <boot order='2'/> <encryption type='...'> ... </encryption> @@ -640,6 +644,7 @@ <host name="hostname" port="7000"/> </source> <target dev="hdb" bus="ide"/> + <boot order='1'/> </disk> </devices> ...</pre> @@ -692,6 +697,14 @@ controls the cache mechanism, possible values are "default", "none", "writethrough" and "writeback". <span class="since">Since 0.1.8</span> </dd> + <dt><code>boot</code></dt> + <dd>Specifies that the disk is bootable. The <code>order</code> + attribute determines the order in which devices will be tried during + boot sequence. The per-device <code>boot</code> elements cannot be + used together with general boot elements in + <a href="#elementsOSBIOS">BIOS bootloader</a> section. + <span class="since">Since 0.8.8</span> + </dd> <dt><code>encryption</code></dt> <dd>If present, specifies how the volume is encrypted. See the <a href="formatstorageencryption.html">Storage Encryption</a> page @@ -813,6 +826,7 @@ <source bridge='xenbr0'/> <mac address='00:16:3e:5d:c7:9e'/> <script path='vif-bridge'/> + <boot order='1'/> </interface> </devices> ...</pre> @@ -1090,6 +1104,29 @@ qemu-kvm -net nic,model=? /dev/null ignored. </p> + <h5><a name="elementsNICSBoot">Specifying boot order</a></h5> + +<pre> + ... + <devices> + <interface type='network'> + <source network='default'/> + <target dev='vnet1'/> + <b><boot order='1'/></b> + </interface> + </devices> + ...</pre> + + <p> + For hypervisors which support this, you can set exact NIC which should + be used for network boot. The <code>order</code> attribute determines + the order in which devices will be tried during boot sequence. The + per-device <code>boot</code> elements cannot be used together with + general boot elements in + <a href="#elementsOSBIOS">BIOS bootloader</a> section. + <span class="since">Since 0.8.8</span> + </p> + <h4><a name="elementsInput">Input devices</a></h4> <p> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 6de85fd..8e2027f 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -121,9 +121,9 @@ </optional> <choice> <ref name="osbootkernel"/> - <oneOrMore> + <zeroOrMore> <ref name="osbootdev"/> - </oneOrMore> + </zeroOrMore> </choice> <optional> <element name="bootmenu"> @@ -526,6 +526,9 @@ </optional> <ref name="target"/> <optional> + <ref name="deviceBoot"/> + </optional> + <optional> <element name="readonly"> <empty/> </element> @@ -963,6 +966,7 @@ - the IP address bound to the interface - the name of the script used to set up the binding - the target device used + - boot order --> <define name="interface-options"> <interleave> @@ -1012,6 +1016,9 @@ <ref name="filterref-node-attributes"/> </element> </optional> + <optional> + <ref name="deviceBoot"/> + </optional> </interleave> </define> <define name="virtualPortProfile"> @@ -1985,6 +1992,15 @@ </optional> </define> + <define name="deviceBoot"> + <element name="boot"> + <attribute name="order"> + <ref name="positiveInteger"/> + </attribute> + <empty/> + </element> + </define> + <!-- Optional hypervisor extensions in their own namespace: QEmu diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 99d5a56..cb9113c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -782,7 +782,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) if (STREQ(caps->guests[i]->features[j]->name, "pae") || STREQ(caps->guests[i]->features[j]->name, "nonpae") || STREQ(caps->guests[i]->features[j]->name, "ia64_be") || - STREQ(caps->guests[i]->features[j]->name, "cpuselection")) { + STREQ(caps->guests[i]->features[j]->name, "cpuselection") || + STREQ(caps->guests[i]->features[j]->name, "deviceboot")) { virBufferVSprintf(&xml, " <%s/>\n", caps->guests[i]->features[j]->name); } else { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7c7a6fa..ec098f6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1511,6 +1511,35 @@ cleanup: } static int +virDomainDeviceBootParseXML(xmlNodePtr node, + int *bootIndex) +{ + char *order; + int boot; + int ret = -1; + + order = virXMLPropString(node, "order"); + if (!order) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing boot order attribute")); + goto cleanup; + } else if (virStrToLong_i(order, NULL, 10, &boot) < 0 || + boot <= 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("incorrect boot order '%s', expecting positive integer"), + order); + goto cleanup; + } + + *bootIndex = boot; + ret = 0; + +cleanup: + VIR_FREE(order); + return ret; +} + +static int virDomainParseLegacyDeviceAddress(char *devaddr, virDomainDevicePCIAddressPtr pci) { @@ -1741,6 +1770,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, } else if ((serial == NULL) && (xmlStrEqual(cur->name, BAD_CAST "serial"))) { serial = (char *)xmlNodeGetContent(cur); + } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { + if (virDomainDeviceBootParseXML(cur, &def->bootIndex)) + goto error; } } cur = cur->next; @@ -2380,6 +2412,9 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "state")) { /* Legacy back-compat. Don't add any more attributes here */ devaddr = virXMLPropString(cur, "devaddr"); + } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { + if (virDomainDeviceBootParseXML(cur, &def->bootIndex)) + goto error; } } cur = cur->next; @@ -4591,6 +4626,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, int i, n; char *bootstr; int ret = -1; + bool deviceBoot; + + deviceBoot = virXPathBoolean("boolean(./devices/*/boot)", ctxt) > 0; /* analysis of the boot devices */ if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { @@ -4599,6 +4637,13 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, goto cleanup; } + if (n > 0 && deviceBoot) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("per-device boot elements cannot be used" + " together with os/boot elements")); + goto cleanup; + } + for (i = 0 ; i < n && i < VIR_DOMAIN_BOOT_LAST ; i++) { int val; char *dev = virXMLPropString(nodes[i], "dev"); @@ -4617,7 +4662,7 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, VIR_FREE(dev); def->os.bootDevs[def->os.nBootDevs++] = val; } - if (def->os.nBootDevs == 0) { + if (def->os.nBootDevs == 0 && !deviceBoot) { def->os.nBootDevs = 1; def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; } @@ -6068,6 +6113,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferVSprintf(buf, " <target dev='%s' bus='%s'/>\n", def->dst, bus); + if (def->bootIndex) + virBufferVSprintf(buf, " <boot order='%d'/>\n", def->bootIndex); if (def->readonly) virBufferAddLit(buf, " <readonly/>\n"); if (def->shared) @@ -6307,6 +6354,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferVSprintf(buf, ">\n%s </filterref>\n", attrs); VIR_FREE(attrs); } + if (def->bootIndex) + virBufferVSprintf(buf, " <boot order='%d'/>\n", def->bootIndex); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6a8ec64..c5dc4bd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -197,6 +197,7 @@ struct _virDomainDiskDef { char *serial; int cachemode; int error_policy; + int bootIndex; unsigned int readonly : 1; unsigned int shared : 1; virDomainDeviceInfo info; @@ -338,6 +339,7 @@ struct _virDomainNetDef { } direct; } data; char *ifname; + int bootIndex; virDomainDeviceInfo info; char *filter; virNWFilterHashTablePtr filterparams; -- 1.7.4.rc1
On 01/14/2011 10:04 AM, Jiri Denemark wrote:
Currently, boot order can be specified per device class but there is no way to specify exact disk/NIC device to boot from.
This patch adds <boot order='N'/> element which can be used inside <disk/> and <interface/>. This is incompatible with the older os/boot element. Since not all hypervisors support per-device boot specification, new deviceboot flag is included in capabilities XML for hypervisors which understand the new boot element. Presence of the flag allows (but doesn't require) users to use the new style boot order specification.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Jan 14, 2011 at 06:04:44PM +0100, Jiri Denemark wrote:
Currently, boot order can be specified per device class but there is no way to specify exact disk/NIC device to boot from.
This patch adds <boot order='N'/> element which can be used inside <disk/> and <interface/>. This is incompatible with the older os/boot element. Since not all hypervisors support per-device boot specification, new deviceboot flag is included in capabilities XML for hypervisors which understand the new boot element. Presence of the flag allows (but doesn't require) users to use the new style boot order specification. --- docs/formatcaps.html.in | 1 + docs/formatdomain.html.in | 41 ++++++++++++++++++++++++++++++++++- docs/schemas/domain.rng | 20 +++++++++++++++- src/conf/capabilities.c | 3 +- src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 + 6 files changed, 112 insertions(+), 6 deletions(-)
ACK Daniel
Support for this is included in qemu and seabios from upstream git. --- Notes: Version 2: - support for floppy devices - rebased on top of Eric's capabilities changes src/qemu/qemu_capabilities.c | 9 +++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 ++++++++++++++++++++---- src/qemu/qemu_command.h | 6 ++++-- src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 8 ++++---- 6 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f967255..1f0a3c2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -431,6 +431,7 @@ qemuCapsInitGuest(virCapsPtr caps, int nmachines = 0; struct stat st; unsigned int ncpus; + unsigned long long qemuCmdFlags; int ret = -1; /* Check for existance of base emulator, or alternate base @@ -546,6 +547,11 @@ qemuCapsInitGuest(virCapsPtr caps, !virCapabilitiesAddGuestFeature(guest, "cpuselection", 1, 0)) goto error; + if (qemuCapsExtractVersionInfo(binary, NULL, &qemuCmdFlags) < 0 || + ((qemuCmdFlags & QEMUD_CMD_FLAG_BOOTINDEX) && + !virCapabilitiesAddGuestFeature(guest, "deviceboot", 1, 0))) + goto error; + if (hvm) { if (virCapabilitiesAddGuestDomain(guest, "qemu", @@ -1047,6 +1053,7 @@ qemuCapsExtractDeviceStr(const char *qemu, * '-device ?'. */ cmd = virCommandNewArgList(qemu, "-device", "pci-assign,?", + "-device", "virtio-blk-pci,?", NULL); virCommandAddEnvPassCommon(cmd); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ @@ -1070,6 +1077,8 @@ qemuCapsParseDeviceStr(const char *str, unsigned long long *flags) { if (strstr(str, "pci-assign.configfd")) *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + if (strstr(str, "virtio-blk-pci.bootindex")) + *flags |= QEMUD_CMD_FLAG_BOOTINDEX; return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8057479..e9e2da0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -83,6 +83,7 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_SPICE = (1LL << 46), /* Is -spice avail */ QEMUD_CMD_FLAG_VGA_NONE = (1LL << 47), /* The 'none' arg for '-vga' */ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD = (1LL << 48), /* -incoming fd:n */ + QEMUD_CMD_FLAG_BOOTINDEX = (1LL << 49), /* -device bootindex property */ }; virCapsPtr qemuCapsInit(virCapsPtr old_caps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 86c5bb5..b9d1140 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1308,7 +1308,8 @@ error: char * -qemuBuildDriveDevStr(virDomainDiskDefPtr disk) +qemuBuildDriveDevStr(virDomainDiskDefPtr disk, + unsigned long long qemuCmdFlags) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); @@ -1348,6 +1349,8 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk) } virBufferVSprintf(&opt, ",drive=%s%s", QEMU_DRIVE_HOST_PREFIX, disk->info.alias); virBufferVSprintf(&opt, ",id=%s", disk->info.alias); + if (disk->bootIndex && (qemuCmdFlags & QEMUD_CMD_FLAG_BOOTINDEX)) + virBufferVSprintf(&opt, ",bootindex=%d", disk->bootIndex); if (virBufferError(&opt)) { virReportOOMError(); @@ -1504,7 +1507,9 @@ qemuBuildNicStr(virDomainNetDefPtr net, char * -qemuBuildNicDevStr(virDomainNetDefPtr net, int vlan) +qemuBuildNicDevStr(virDomainNetDefPtr net, + int vlan, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; const char *nic; @@ -1529,6 +1534,8 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, int vlan) net->mac[4], net->mac[5]); if (qemuBuildDeviceAddressStr(&buf, &net->info) < 0) goto error; + if (net->bootIndex && (qemuCmdFlags & QEMUD_CMD_FLAG_BOOTINDEX)) + virBufferVSprintf(&buf, ",bootindex=%d", net->bootIndex); if (virBufferError(&buf)) { virReportOOMError(); @@ -3082,10 +3089,19 @@ qemuBuildCommandLine(virConnectPtr conn, disk->info.addr.drive.unit ? 'B' : 'A', disk->info.alias); + + if (disk->bootIndex && + (qemuCmdFlags & QEMUD_CMD_FLAG_BOOTINDEX)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.bootindex%c=%d", + disk->info.addr.drive.unit + ? 'B' : 'A', + disk->bootIndex); + } } else { virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildDriveDevStr(disk))) + if (!(optstr = qemuBuildDriveDevStr(disk, qemuCmdFlags))) goto error; virCommandAddArg(cmd, optstr); VIR_FREE(optstr); @@ -3306,7 +3322,7 @@ qemuBuildCommandLine(virConnectPtr conn, } if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { virCommandAddArg(cmd, "-device"); - if (!(nic = qemuBuildNicDevStr(net, vlan))) + if (!(nic = qemuBuildNicDevStr(net, vlan, qemuCmdFlags))) goto error; virCommandAddArg(cmd, nic); VIR_FREE(nic); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4c42a10..9e9d5f5 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -62,7 +62,8 @@ char * qemuBuildNicStr(virDomainNetDefPtr net, /* Current, best practice */ char * qemuBuildNicDevStr(virDomainNetDefPtr net, - int vlan); + int vlan, + unsigned long long qemuCmdFlags); char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, unsigned long long qemuCmdFlags); @@ -75,7 +76,8 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, unsigned long long qemuCmdFlags); /* Current, best practice */ -char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk); +char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk, + unsigned long long qemuCmdFlags); char * qemuBuildFSDevStr(virDomainFSDefPtr fs); /* Current, best practice */ char * qemuBuildControllerDevStr(virDomainControllerDefPtr def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..cfb56bc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6054,6 +6054,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, */ for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; + int bootIndex = net->bootIndex; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { VIR_FREE(net->data.network.name); @@ -6076,6 +6077,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, net->data.ethernet.script = script; net->data.ethernet.ipaddr = ipaddr; } + net->bootIndex = bootIndex; } for (i = 0 ; i < def->ngraphics ; i++) { if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1dc036c..3b4a673 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -169,7 +169,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk))) + if (!(devstr = qemuBuildDriveDevStr(disk, qemuCmdFlags))) goto error; } @@ -380,7 +380,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuAssignDeviceDiskAlias(disk, qemuCmdFlags) < 0) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk))) + if (!(devstr = qemuBuildDriveDevStr(disk, qemuCmdFlags))) goto error; } @@ -493,7 +493,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, goto error; if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk))) + if (!(devstr = qemuBuildDriveDevStr(disk, qemuCmdFlags))) goto error; } @@ -675,7 +675,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, } if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { - if (!(nicstr = qemuBuildNicDevStr(net, vlan))) + if (!(nicstr = qemuBuildNicDevStr(net, vlan, qemuCmdFlags))) goto try_remove; } else { if (!(nicstr = qemuBuildNicStr(net, NULL, vlan))) -- 1.7.4.rc1
On 01/14/2011 10:04 AM, Jiri Denemark wrote:
Support for this is included in qemu and seabios from upstream git.
--- Notes: Version 2: - support for floppy devices - rebased on top of Eric's capabilities changes
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8057479..e9e2da0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -83,6 +83,7 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_SPICE = (1LL << 46), /* Is -spice avail */ QEMUD_CMD_FLAG_VGA_NONE = (1LL << 47), /* The 'none' arg for '-vga' */ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD = (1LL << 48), /* -incoming fd:n */ + QEMUD_CMD_FLAG_BOOTINDEX = (1LL << 49), /* -device bootindex property */
Lots of us wanting to touch this file lately - whoever commits second better remember to resolve the conflict correctly :) Overall, looks good to me (unless I'm overlooking something about qemu behavior that Dan is more familiar with). ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Jan 14, 2011 at 06:04:45PM +0100, Jiri Denemark wrote:
Support for this is included in qemu and seabios from upstream git.
--- Notes: Version 2: - support for floppy devices - rebased on top of Eric's capabilities changes
src/qemu/qemu_capabilities.c | 9 +++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 ++++++++++++++++++++---- src/qemu/qemu_command.h | 6 ++++-- src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 8 ++++---- 6 files changed, 40 insertions(+), 10 deletions(-)
ACK Daniel
--- Notes: Version 2: - support for floppy devices .../qemuxml2argvdata/qemuxml2argv-boot-order.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 52 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 4 files changed, 56 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args new file mode 100644 index 0000000..8536efa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=sheepdog:example.org:6000:image,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=3 -drive file=/root/boot.iso,if=none,media=cdrom,id=drive-ide0-1-0 -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 -drive file=/dev/null,if=none,id=drive-fdc0-0-1 -global isa-fdc.driveB=drive-fdc0-0-1 -global isa-fdc.bootindexB=2 -device virtio-net-pci,vlan=0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x2,bootindex=2 -net user,vlan=0,name=hostnet0 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml new file mode 100644 index 0000000..73049d9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml @@ -0,0 +1,52 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='sheepdog' name='image'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + <boot order='3'/> + </disk> + <disk type='file' device='cdrom'> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + <boot order='1'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <disk type='file' device='floppy'> + <driver name='qemu' type='raw'/> + <source file='/dev/null'/> + <target dev='fdb' bus='fdc'/> + <boot order='2'/> + <address type='drive' controller='0' bus='0' unit='1'/> + </disk> + <controller type='ide' index='0'/> + <controller type='fdc' index='0'/> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + <boot order='2'/> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6760f67..a0e4481 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -252,6 +252,8 @@ mymain(int argc, char **argv) DO_TEST("boot-floppy", 0, false); DO_TEST("boot-multi", QEMUD_CMD_FLAG_BOOT_MENU, false); DO_TEST("boot-menu-disable", QEMUD_CMD_FLAG_BOOT_MENU, false); + DO_TEST("boot-order", QEMUD_CMD_FLAG_BOOTINDEX | + QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DEVICE, false); DO_TEST("bootloader", QEMUD_CMD_FLAG_DOMID, true); DO_TEST("clock-utc", 0, false); DO_TEST("clock-localtime", 0, false); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 326a1f1..ab82d36 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -127,6 +127,7 @@ mymain(int argc, char **argv) DO_TEST("boot-floppy"); DO_TEST("boot-multi"); DO_TEST("boot-menu-disable"); + DO_TEST("boot-order"); DO_TEST("bootloader"); DO_TEST("clock-utc"); DO_TEST("clock-localtime"); -- 1.7.4.rc1
On 01/14/2011 10:04 AM, Jiri Denemark wrote:
--- Notes: Version 2: - support for floppy devices
+ <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='sheepdog' name='image'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + <boot order='3'/> + </disk>
Just to make sure I'm clear - if any <disk> has a <boot> sub-element, then all remaining disks (in this case, /dev/HostVG/QEMUGuest1) that lack <boot> are not even considered for booting within the guest bios. What happens if <boot order='1'/> is accidentally specified twice among two different <disk>/<interface> elements? Or what if I only supply <boot order='2'/>, but no 1? Does this series need an additional patch that ensures that there are no duplicates, or even that once all devices are visited, the boot order is contiguous from 1 to n?
+++ b/tests/qemuxml2xmltest.c @@ -127,6 +127,7 @@ mymain(int argc, char **argv) DO_TEST("boot-floppy"); DO_TEST("boot-multi"); DO_TEST("boot-menu-disable"); + DO_TEST("boot-order");
Oh, good point - my smartcard patch forgot to update this file (there's probably several cases where we touched one but not all related test files). ACK to this patch, but I'm not sure your series is complete yet. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Jan 14, 2011 at 11:51:56 -0700, Eric Blake wrote:
On 01/14/2011 10:04 AM, Jiri Denemark wrote:
+ <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='sheepdog' name='image'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + <boot order='3'/> + </disk>
Just to make sure I'm clear - if any <disk> has a <boot> sub-element, then all remaining disks (in this case, /dev/HostVG/QEMUGuest1) that lack <boot> are not even considered for booting within the guest bios.
Devices with <boot> are tried first in the given order. What happens if the boot doesn't succeed after trying all of them is undefined and depends on the guest bios. E.g., seabios in qemu will try a usual set of devices (something like hda, cdrom, nic) in that case. We could make the semantics stricter but I don't think we would be able to enforce that.
What happens if <boot order='1'/> is accidentally specified twice among two different <disk>/<interface> elements? Or what if I only supply <boot order='2'/>, but no 1?
In current state of the patches, this depends on the hypervisor...
Does this series need an additional patch that ensures that there are no duplicates, or even that once all devices are visited, the boot order is contiguous from 1 to n?
Yeah, I it makes sense. Although I think we should just check and fail if there are duplicates or if the boot order is not from 1 to n instead of changing the order automagically. I'll prepare a follow-up patch for this. Jirka
On Fri, Jan 14, 2011 at 06:04:46PM +0100, Jiri Denemark wrote:
--- Notes: Version 2: - support for floppy devices
.../qemuxml2argvdata/qemuxml2argv-boot-order.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 52 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 4 files changed, 56 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml
ACK Daniel
Since I haven't pushed the series yet, I can easily squash the following patch into patch 2/4. Jirka
From 5302f9ca43e26bd23230ba84e169a0e9109d04e8 Mon Sep 17 00:00:00 2001 Message-Id: <5302f9ca43e26bd23230ba84e169a0e9109d04e8.1295268171.git.jdenemar@redhat.com> From: Jiri Denemark <jdenemar@redhat.com> Date: Mon, 17 Jan 2011 13:25:31 +0100 Subject: [PATCH] Check that boot order sequence is correct Mail-Followup-To: libvir-list@redhat.com
The boot order sequence has to be contiguous starting from 1 and without duplicates. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf8c5fa..645767e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -46,6 +46,7 @@ #include "ignore-value.h" #include "storage_file.h" #include "files.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -1531,7 +1532,8 @@ cleanup: static int virDomainDeviceBootParseXML(xmlNodePtr node, - int *bootIndex) + int *bootIndex, + virBitmapPtr bootMap) { char *order; int boot; @@ -1550,6 +1552,20 @@ virDomainDeviceBootParseXML(xmlNodePtr node, goto cleanup; } + if (bootMap) { + bool set; + if (virBitmapGetBit(bootMap, boot - 1, &set) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("boot orders have to be contiguous and starting from 1")); + goto cleanup; + } else if (set) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("boot order %d used for more than one device"), boot); + goto cleanup; + } + ignore_value(virBitmapSetBit(bootMap, boot - 1)); + } + *bootIndex = boot; ret = 0; @@ -1643,7 +1659,9 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, - int flags) { + virBitmapPtr bootMap, + int flags) +{ virDomainDiskDefPtr def; xmlNodePtr cur, host; char *type = NULL; @@ -1790,7 +1808,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, (xmlStrEqual(cur->name, BAD_CAST "serial"))) { serial = (char *)xmlNodeGetContent(cur); } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { - if (virDomainDeviceBootParseXML(cur, &def->bootIndex)) + if (virDomainDeviceBootParseXML(cur, &def->bootIndex, + bootMap)) goto error; } } @@ -2329,7 +2348,9 @@ static virDomainNetDefPtr virDomainNetDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, - int flags ATTRIBUTE_UNUSED) { + virBitmapPtr bootMap, + int flags ATTRIBUTE_UNUSED) +{ virDomainNetDefPtr def; xmlNodePtr cur; char *macaddr = NULL; @@ -2440,7 +2461,8 @@ virDomainNetDefParseXML(virCapsPtr caps, /* Legacy back-compat. Don't add any more attributes here */ devaddr = virXMLPropString(cur, "devaddr"); } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { - if (virDomainDeviceBootParseXML(cur, &def->bootIndex)) + if (virDomainDeviceBootParseXML(cur, &def->bootIndex, + bootMap)) goto error; } } @@ -4429,7 +4451,8 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, flags))) + if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, + NULL, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) { dev->type = VIR_DOMAIN_DEVICE_FS; @@ -4437,7 +4460,8 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "interface")) { dev->type = VIR_DOMAIN_DEVICE_NET; - if (!(dev->data.net = virDomainNetDefParseXML(caps, node, ctxt, flags))) + if (!(dev->data.net = virDomainNetDefParseXML(caps, node, ctxt, + NULL, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "input")) { dev->type = VIR_DOMAIN_DEVICE_INPUT; @@ -4708,15 +4732,21 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, static int virDomainDefParseBootXML(xmlXPathContextPtr ctxt, - virDomainDefPtr def) + virDomainDefPtr def, + unsigned long *bootCount) { xmlNodePtr *nodes = NULL; int i, n; char *bootstr; int ret = -1; - bool deviceBoot; + unsigned long deviceBoot; - deviceBoot = virXPathBoolean("boolean(./devices/*/boot)", ctxt) > 0; + if (virXPathULong("count(./devices/disk[boot]" + "|./devices/interface[boot])", ctxt, &deviceBoot) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot count boot devices")); + goto cleanup; + } /* analysis of the boot devices */ if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { @@ -4764,6 +4794,7 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, VIR_FREE(bootstr); } + *bootCount = deviceBoot; ret = 0; cleanup: @@ -4784,6 +4815,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDefPtr def; unsigned long count; bool uuid_generated = false; + virBitmapPtr bootMap = NULL; + unsigned long bootMapSize = 0; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -5100,9 +5133,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.loader = virXPathString("string(./os/loader[1])", ctxt); } - if (STREQ(def->os.type, "hvm") && - virDomainDefParseBootXML(ctxt, def) < 0) { - goto error; + if (STREQ(def->os.type, "hvm")) { + if (virDomainDefParseBootXML(ctxt, def, &bootMapSize) < 0) + goto error; + if (bootMapSize && !(bootMap = virBitmapAlloc(bootMapSize))) + goto no_memory; } def->emulator = virXPathString("string(./devices/emulator[1])", ctxt); @@ -5123,6 +5158,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], + bootMap, flags); if (!disk) goto error; @@ -5179,6 +5215,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainNetDefPtr net = virDomainNetDefParseXML(caps, nodes[i], ctxt, + bootMap, flags); if (!net) goto error; @@ -5578,6 +5615,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (virDomainDefAddImplicitControllers(def) < 0) goto error; + virBitmapFree(bootMap); + return def; no_memory: @@ -5587,6 +5626,7 @@ no_memory: error: VIR_FREE(tmp); VIR_FREE(nodes); + virBitmapFree(bootMap); virDomainDefFree(def); return NULL; } -- 1.7.4.rc2
On 01/17/2011 05:46 AM, Jiri Denemark wrote:
Since I haven't pushed the series yet, I can easily squash the following patch into patch 2/4.
Jirka
From 5302f9ca43e26bd23230ba84e169a0e9109d04e8 Mon Sep 17 00:00:00 2001 Message-Id: <5302f9ca43e26bd23230ba84e169a0e9109d04e8.1295268171.git.jdenemar@redhat.com> From: Jiri Denemark <jdenemar@redhat.com> Date: Mon, 17 Jan 2011 13:25:31 +0100 Subject: [PATCH] Check that boot order sequence is correct Mail-Followup-To: libvir-list@redhat.com
The boot order sequence has to be contiguous starting from 1 and without duplicates. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 53 insertions(+), 13 deletions(-)
ACK - looks like a nice use of a bitmap for the solution. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Jan 17, 2011 at 01:46:09PM +0100, Jiri Denemark wrote:
Since I haven't pushed the series yet, I can easily squash the following patch into patch 2/4.
Jirka
From 5302f9ca43e26bd23230ba84e169a0e9109d04e8 Mon Sep 17 00:00:00 2001 Message-Id: <5302f9ca43e26bd23230ba84e169a0e9109d04e8.1295268171.git.jdenemar@redhat.com> From: Jiri Denemark <jdenemar@redhat.com> Date: Mon, 17 Jan 2011 13:25:31 +0100 Subject: [PATCH] Check that boot order sequence is correct Mail-Followup-To: libvir-list@redhat.com
The boot order sequence has to be contiguous starting from 1 and without duplicates. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 53 insertions(+), 13 deletions(-)
ACK Daniel
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark