On 09/28/2012 11:56 AM, Kevin Wolf wrote: > From: Jeff Cody <jc...@redhat.com> > > 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 ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
