Re: [Qemu-devel] [PATCH] block: add drive_backup HMP command

[Stefan Hajnoczi]

On Tue, Jun 25, 2013 at 04:36:53PM +0200, Paolo Bonzini wrote:
> Il 25/06/2013 16:06, Kevin Wolf ha scritto:
> > Am 25.06.2013 um 15:49 hat Paolo Bonzini geschrieben:
> >> Il 25/06/2013 15:26, Kevin Wolf ha scritto:
> >>>>> +    if (!full) {
> >>>>> +        error_setg(&errp, "-f is not yet implemented");
> >>>>> +        hmp_handle_error(mon, &errp);
> >>>>> +        return;
> >>>>> +    }
> >>> Then why make it a valid option and confuse users in the help text by
> >>> describing options that don't really exist?
> >>
> >> Because otherwise we're stuck with a meaning of the flag that is
> >> different between drive-mirror and block-backup.
> > 
> > Do you mean when "otherwise" isn't only "we don't add -f now", but also
> > "we accidentally add a -f with different meaning later"? Not sure if
> > there's a real danger of that when we're aware that we want -f with the
> > same meaning as for mirroring.
> 
> We have drive-mirror with:
> * the default is 'top'
> * -f gives 'full'
> 
> block-backup for now only implements 'full'.  If we do not force the
> user to add -f, the default is 'full' and we should not change it later.
> 
> However, I would move the "not yet implemented" error from HMP to QMP.
> This way, both drive-mirror and block-backup will have a mandatory
> 'sync' argument.  We plan to implement it anyway, and it makes sense imo
> to avoid gratuitous differences in the APIs.

Thanks, I should have explained this in the commit message.  Requiring
-f now avoids changing semantics later when 'top' becomes the default to
match drive-mirror.

I'll move the error into qmp_drive_backup().

Stefan