[libvirt] [PATCH] conf: Allow for non-contiguous device boot orders

[Jiri Denemark]

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