[libvirt] [PATCH] conf: Allow for non-contiguous device boot orders
This patch adds the ability to configure non-contiguous boot orders on boot devices. This allows unplugging devices that have boot order specified without breaking migration. The new code now uses a slightly less memory efficient approach to store the boot order fields in a hashtable instead of a bitmap. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc26f21..80a90e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2876,7 +2876,7 @@ cleanup: static int virDomainDeviceBootParseXML(xmlNodePtr node, int *bootIndex, - virBitmapPtr bootMap) + virHashTablePtr bootHash) { char *order; int boot; @@ -2895,18 +2895,19 @@ virDomainDeviceBootParseXML(xmlNodePtr node, goto cleanup; } - if (bootMap) { - bool set; - if (virBitmapGetBit(bootMap, boot - 1, &set) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("boot orders have to be contiguous and starting from 1")); + if (bootHash) { + if (virHashLookup(bootHash, order)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("boot order '%s' used for more than one device"), + order); goto cleanup; - } else if (set) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("boot order %d used for more than one device"), boot); + } + + if (virHashAddEntry(bootHash, order, (void *) 1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to add element to boot order hash")); goto cleanup; } - ignore_value(virBitmapSetBit(bootMap, boot - 1)); } *bootIndex = boot; @@ -2922,7 +2923,7 @@ cleanup: */ static int virDomainDeviceInfoParseXML(xmlNodePtr node, - virBitmapPtr bootMap, + virHashTablePtr bootHash, virDomainDeviceInfoPtr info, unsigned int flags) { @@ -2973,7 +2974,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, } if (boot) { - if (virDomainDeviceBootParseXML(boot, &info->bootIndex, bootMap)) + if (virDomainDeviceBootParseXML(boot, &info->bootIndex, bootHash)) goto cleanup; } @@ -3945,7 +3946,7 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, - virBitmapPtr bootMap, + virHashTablePtr bootHash, virSecurityLabelDefPtr* vmSeclabels, int nvmSeclabels, unsigned int flags) @@ -4660,7 +4661,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, } def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } else { - if (virDomainDeviceInfoParseXML(node, bootMap, &def->info, + if (virDomainDeviceInfoParseXML(node, bootHash, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) goto error; } @@ -5318,7 +5319,7 @@ static virDomainNetDefPtr virDomainNetDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, - virBitmapPtr bootMap, + virHashTablePtr bootHash, unsigned int flags) { virDomainNetDefPtr def; @@ -5513,7 +5514,7 @@ virDomainNetDefParseXML(virCapsPtr caps, } def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } else { - if (virDomainDeviceInfoParseXML(node, bootMap, &def->info, + if (virDomainDeviceInfoParseXML(node, bootHash, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) goto error; @@ -7988,7 +7989,7 @@ error: static virDomainHostdevDefPtr virDomainHostdevDefParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt, - virBitmapPtr bootMap, + virHashTablePtr bootHash, unsigned int flags) { virDomainHostdevDefPtr def; @@ -8029,7 +8030,7 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, } if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (virDomainDeviceInfoParseXML(node, bootMap, def->info, + if (virDomainDeviceInfoParseXML(node, bootHash, def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) goto error; @@ -8063,7 +8064,7 @@ error: static virDomainRedirdevDefPtr virDomainRedirdevDefParseXML(const xmlNodePtr node, - virBitmapPtr bootMap, + virHashTablePtr bootHash, unsigned int flags) { xmlNodePtr cur; @@ -8113,7 +8114,7 @@ virDomainRedirdevDefParseXML(const xmlNodePtr node, def->source.chr.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR; } - if (virDomainDeviceInfoParseXML(node, bootMap, &def->info, + if (virDomainDeviceInfoParseXML(node, bootHash, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) goto error; @@ -9098,8 +9099,7 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, static int virDomainDefParseBootXML(xmlXPathContextPtr ctxt, - virDomainDefPtr def, - unsigned long *bootCount) + virDomainDefPtr def) { xmlNodePtr *nodes = NULL; int i, n; @@ -9196,7 +9196,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, def->os.bios.rt_set = true; } - *bootCount = deviceBoot; ret = 0; cleanup: @@ -9394,8 +9393,7 @@ virDomainDefParseXML(virCapsPtr caps, virDomainDefPtr def; unsigned long count; bool uuid_generated = false; - virBitmapPtr bootMap = NULL; - unsigned long bootMapSize = 0; + virHashTablePtr bootHash = NULL; xmlNodePtr cur; bool usb_none = false; bool usb_other = false; @@ -10289,10 +10287,10 @@ virDomainDefParseXML(virCapsPtr caps, } if (STREQ(def->os.type, "hvm")) { - if (virDomainDefParseBootXML(ctxt, def, &bootMapSize) < 0) + if (virDomainDefParseBootXML(ctxt, def) < 0) + goto error; + if (!(bootHash = virHashCreate(5, NULL))) goto error; - if (bootMapSize && !(bootMap = virBitmapNew(bootMapSize))) - goto no_memory; } def->emulator = virXPathString("string(./devices/emulator[1])", ctxt); @@ -10313,7 +10311,7 @@ virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], ctxt, - bootMap, + bootHash, def->seclabels, def->nseclabels, flags); @@ -10413,7 +10411,7 @@ virDomainDefParseXML(virCapsPtr caps, virDomainNetDefPtr net = virDomainNetDefParseXML(caps, nodes[i], ctxt, - bootMap, + bootHash, flags); if (!net) goto error; @@ -10796,7 +10794,7 @@ virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainHostdevDefPtr hostdev; - hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootMap, flags); + hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootHash, flags); if (!hostdev) goto error; @@ -10912,7 +10910,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { virDomainRedirdevDefPtr redirdev = virDomainRedirdevDefParseXML(nodes[i], - bootMap, + bootHash, flags); if (!redirdev) goto error; @@ -11029,7 +11027,7 @@ virDomainDefParseXML(virCapsPtr caps, if (virDomainDefAddImplicitControllers(def) < 0) goto error; - virBitmapFree(bootMap); + virHashFree(bootHash); return def; @@ -11038,7 +11036,7 @@ no_memory: error: VIR_FREE(tmp); VIR_FREE(nodes); - virBitmapFree(bootMap); + virHashFree(bootHash); virDomainDefFree(def); return NULL; } -- 1.8.1.5
On 04/04/13 15:35, Peter Krempa wrote:
This patch adds the ability to configure non-contiguous boot orders on boot devices. This allows unplugging devices that have boot order specified without breaking migration.
The new code now uses a slightly less memory efficient approach to store the boot order fields in a hashtable instead of a bitmap. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 34 deletions(-)
Ping? Could somebody please have a look. Thanks. Peter
On Thu, Apr 04, 2013 at 15:35:57 +0200, Peter Krempa wrote:
This patch adds the ability to configure non-contiguous boot orders on boot devices. This allows unplugging devices that have boot order specified without breaking migration.
The new code now uses a slightly less memory efficient approach to store the boot order fields in a hashtable instead of a bitmap. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc26f21..80a90e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c ... @@ -2895,18 +2895,19 @@ virDomainDeviceBootParseXML(xmlNodePtr node, goto cleanup; }
- if (bootMap) { - bool set; - if (virBitmapGetBit(bootMap, boot - 1, &set) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("boot orders have to be contiguous and starting from 1")); + if (bootHash) { + if (virHashLookup(bootHash, order)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("boot order '%s' used for more than one device"), + order); goto cleanup; - } else if (set) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("boot order %d used for more than one device"), boot); + } + + if (virHashAddEntry(bootHash, order, (void *) 1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to add element to boot order hash"));
This virReportError is overwriting the error reported by virHashAddEntry. Although virHashAddEntry is pretty stupid in not reporting the error in all cases, it reports them in all paths you can get from here.
goto cleanup; } - ignore_value(virBitmapSetBit(bootMap, boot - 1)); }
*bootIndex = boot;
...
@@ -10289,10 +10287,10 @@ virDomainDefParseXML(virCapsPtr caps, }
if (STREQ(def->os.type, "hvm")) { - if (virDomainDefParseBootXML(ctxt, def, &bootMapSize) < 0) + if (virDomainDefParseBootXML(ctxt, def) < 0) + goto error; + if (!(bootHash = virHashCreate(5, NULL))) goto error; - if (bootMapSize && !(bootMap = virBitmapNew(bootMapSize))) - goto no_memory; }
Previous version only allocated bootMap when device boot was used while your version allocates the hash everytime even if it won't be used. But I think we don't care enough to change it. ... ACK with the redundant error message removed. Jirka
On 04/12/13 13:46, Jiri Denemark wrote:
On Thu, Apr 04, 2013 at 15:35:57 +0200, Peter Krempa wrote:
This patch adds the ability to configure non-contiguous boot orders on boot devices. This allows unplugging devices that have boot order specified without breaking migration.
The new code now uses a slightly less memory efficient approach to store the boot order fields in a hashtable instead of a bitmap. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 34 deletions(-)
...
...
ACK with the redundant error message removed.
Jirka
I removed the error message and pushed this patch. Thanks! Peter
participants (2)
-
Jiri Denemark -
Peter Krempa