[libvirt] [PATCH v3 3/4] qemu_migration: Send disk sizes to the other side

[Peter Krempa]

On 12/02/14 17:13, Michal Privoznik wrote:
> Up 'til now, users need to precreate non-shared storage on migration
> themselves. This is not very friendly requirement and we should do
> something about it. In this patch, the migration cookie is extended,
> so that <nbd/> section does not only contain NBD port, but info on
> disks being migrated. This patch sends a list of pairs of:
> 
>     <disk target; disk size>
> 
> to the destination. The actual storage allocation is left for next
> commit.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_migration.c | 160 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 140 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index f19e68c..36b7e43 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c

> @@ -891,6 +966,64 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt)
>  }
>  
>  
> +static qemuMigrationCookieNBDPtr
> +qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt)
> +{
> +    qemuMigrationCookieNBDPtr ret = NULL;
> +    char *port = NULL, *capacity = NULL;
> +    size_t i;
> +    int n;
> +    xmlNodePtr *disks = NULL;
> +    xmlNodePtr save_ctxt = ctxt->node;
> +
> +    if (VIR_ALLOC(ret) < 0)
> +        goto error;
> +
> +    port = virXPathString("string(./nbd/@port)", ctxt);
> +    if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Malformed nbd port '%s'"),
> +                       port);
> +        goto error;
> +    }
> +
> +    /* Now check if source sent a list of disks to prealloc. We might be
> +     * talking to an older server, so it's not an error if the list is
> +     * missing. */
> +    if ((n = virXPathNodeSet("./nbd/disk", ctxt, &disks)) > 0) {
> +        if (VIR_ALLOC_N(ret->disks, n) < 0)
> +            goto error;
> +        ret->ndisks = n;
> +
> +        for (i = 0; i < n; i++) {
> +            ctxt->node = disks[i];
> +            VIR_FREE(capacity);
> +
> +            ret->disks[i].target = virXPathString("string(./@target)", ctxt);

Note here that the existence of the @target element is not checked in
case the cookie XML contains <nbd><disk .. entries.

In 4/4 you use the field unconditionally which could lead to a crash
with a specially crafted migration cookie. I think a check and a quick
error message wouldn't hurt here.

> +            capacity = virXPathString("string(./@capacity)", ctxt);
> +            if (virStrToLong_ull(capacity, NULL, 10,

if (!capacity ||

> +                                 &ret->disks[i].capacity) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Malformed disk capacity: '%s'"),
> +                               capacity);

NULLSTR(capacity)

> +                goto error;
> +            }
> +        }
> +    }
> +
> + cleanup:
> +    VIR_FREE(port);
> +    VIR_FREE(capacity);
> +    VIR_FREE(disks);
> +    ctxt->node = save_ctxt;
> +    return ret;
> + error:
> +    qemuMigrationCookieNBDFree(ret);
> +    ret = NULL;
> +    goto cleanup;
> +}
> +
> +
>  static qemuDomainJobInfoPtr
>  qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt)
>  {

ACK with NULL-hardening added.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141202/a9a4bd4b/attachment-0001.sig>