Re: [Qemu-devel] [PATCH V4 4/4] Change default to qcow2 for sync mode none.

[Ian Main]

On Thu, Jul 18, 2013 at 11:27:21AM -0600, Eric Blake wrote:
> On 07/17/2013 02:04 PM, Ian Main wrote:
> > qcow2 supports backing files so it makes sense to default to qcow2
> > for MIRROR_SYNC_MODE_NONE so that we can use the source as a backing
> > drive and export it via nbd.  Defaulting FULL and TOP to SYNC_MODE_NONE
> > breaks tests but that could be fixed if we wanted it.
> > 
> > Signed-off-by: Ian Main <address@hidden>
> > ---
> >  blockdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 000dea6..805b0e5 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -1462,7 +1462,7 @@ void qmp_drive_backup(const char *device, const char 
> > *target,
> >      }
> >  
> >      if (!has_format) {
> > -        format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : 
> > bs->drv->format_name;
> > +        format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : "qcow2";
> 
> Is this the right thing to do?  Or should we do:
> 
> if (!has_format) {
>     if (mode == NEW_IMAGE_MODE_EXISTING) {
>         format = NULL;
>     } else {
>         format = bs->drv->format_name ?: "qcow2";
>     }
> }
> 
> That is, I think we should default to doing a backup in the format given
> by the original (what if the original is qed, which also supports
> backing files), and only use qcow2 when there is no guidance whatsoever.
> 
> But in practice, I don't care - format probing is a security hole, so
> libvirt should always be passing a format, at which point the entire
> !has_format condition should be skipped when called by libvirt.

Heh, actually that is basically what I have now, as with the doc change
I forgot to git add it.  Doh!  I'll repost.. I'm also not opposed to
format being non-optional.

        Ian