[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 12/30] QAPI: add command for live block commit,
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 12/30] QAPI: add command for live block commit, 'block-commit' |
Date: |
Thu, 11 Oct 2012 09:42:46 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
On 09/28/2012 11:56 AM, Kevin Wolf wrote:
> From: Jeff Cody <address@hidden>
>
> The command for live block commit is added, which has the following
> arguments:
>
> + /* default top_bs is the active layer */
> + top_bs = bs;
> +
> + if (top) {
> + if (strcmp(bs->filename, top) != 0) {
> + top_bs = bdrv_find_backing_image(bs, top);
> + }
> + }
In light of Jeff's followup patches to fix bdrv_find_backing_image when
dealing with relative file names, I see another bug here (but impact is
limited to only a poor-quality error message, claiming that 'top was not
found' instead of the intended 'commit of an active top is not
supported'). That is, strcmp() on user-supplied 'top' is not guaranteed
to succeed if the user provided a different spelling for the same file
as was actually recorded in bs->filename, and since you have resorted to
realpath() before strcmp() in bdrv_find_backing_image to cope with
alternat user spellings, I think you also need to use realpath() before
comparison here (probably a new helper function bdrv_is_equal() or some
such name, that compares a user-supplied name with a bs). Saving this
until a followup patch as part of your phase 2 series to add active
image commit support is fine by me.
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature