Re: [Qemu-devel] [PATCH V6 1/3] Implement sync modes for drive-backup.

[Ian Main <imain@redhat.com>]

On Wed, Jul 24, 2013 at 02:32:53PM -0600, Eric Blake wrote:
> On 07/24/2013 04:55 AM, Kevin Wolf wrote:
> 
> > Unconditionally overriding format for NEW_IMAGE_MODE_EXISTING is
> > definitely wrong. It's the user's choice which COW format to use for the
> > backup image. There's no reason why it has to be the same format as the
> > image that is being backed up.
> > 
> > Before, bs->drv->format_name was a default for the case where a new
> > image had to be created and no format was given; and the format of
> > existing images could be probed. This is still what makes most sense to
> > me. What's even the goal with this change?

Actually I think that code is wrong.  If we are using
NEW_IMAGE_MODE_EXISTING then format doesn't get used.  We just end up
using bdrv_open() below to open the existing image.  Format should not
be specified for an existing image.
 
> Furthermore, I'm proposing that for 1.6, we should make the format
> argument mandatory for drive-backup.  We made it optional for
> drive-mirror, to allow for probing, but there have been CVEs in the past
> due to probing of a raw file gone wrong.  We can always relax a
> mandatory argument into an optional one in 1.7, if we decide that
> probing can be done safely, but we can never turn an optional argument
> into a mandatory one once the initial release bakes in the option.  It
> would make the code a lot simpler to just have a mandatory format
> argument, instead of having to bake in and document hueristics on which
> format is picked when the caller doesn't provide one.

So I made format mandatory in the last patch but only for
NEW_IMAGE_MODE_ABSOLUTE_PATHS.  It actually doesn't make sense to
specify the format of an existing image so I left it optional as an
argument, but it will throw an error if it's not specified for the case
where we create a new image.

That make sense?

	Ian