On Fri, 09/11 15:26, Eric Blake wrote:
> On 09/07/2015 01:34 AM, Fam Zheng wrote:
> > From: Stefan Hajnoczi <stefa...@redhat.com>
> >
> > Join the transaction when the 'transactional-cancel' QMP argument is
> > true.
> >
> > This ensures that the sync bitmap is not thrown away if another block
> > job in the transaction is cancelled or fails. This is critical so
> > incremental backup with multiple disks can be retried in case of
> > cancellation/failure.
> >
> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
>
> Interface review:
>
> > +void qmp_blockdev_backup(const char *device, const char *target,
> > + enum MirrorSyncMode sync,
> > + bool has_speed, int64_t speed,
> > + bool has_on_source_error,
> > + BlockdevOnError on_source_error,
> > + bool has_on_target_error,
> > + BlockdevOnError on_target_error,
> > + bool has_transactional_cancel,
> > + bool transactional_cancel,
> > + Error **errp)
> > +{
> > + if (has_transactional_cancel && transactional_cancel) {
> > + error_setg(errp, "Transactional cancel can only be used in the "
> > + "'transaction' command");
> > + return;
> > + }
>
> Hmm. It might be nicer if we had two separate qapi structs:
>
> # used in 'blockdev-backup'
> { 'struct':'BlockdevBackup', 'data': { device ... on-target-error } }
>
> # used in 'transaction'
> { 'struct':'BlockdevBackupTxn', 'base':'BlockdevBackup',
> 'data': { 'transactional-cancel':'bool' } }
>
> so that the user can't abuse the boolean from the wrong context.
Very good point, will do that in v6.
Fam
