Re: [RFC 04/15] qapi: block-job-change: make copy-mode parameter optional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx> writes:

> We are going to add more parameters to change. We want to make possible
> to change only one or any subset of available options. So all the
> options should be optional.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx>
> ---
>  block/mirror.c       | 5 +++++
>  qapi/block-core.json | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index a177502e4f..2d0cd22c06 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1265,6 +1265,11 @@ static void mirror_change(BlockJob *job, JobChangeOptions *opts,
>  
>      GLOBAL_STATE_CODE();
>  
> +    if (!change_opts->has_copy_mode) {
> +        /* Nothing to do */

I doubt the comment is useful.

> +        return;
> +    }
> +
>      if (qatomic_read(&s->copy_mode) == change_opts->copy_mode) {
>          return;
>      }

       if (change_opts->copy_mode != MIRROR_COPY_MODE_WRITE_BLOCKING) {
           error_setg(errp, "Change to copy mode '%s' is not implemented",
                      MirrorCopyMode_str(change_opts->copy_mode));
           return;
       }

       current = qatomic_cmpxchg(&s->copy_mode, MIRROR_COPY_MODE_BACKGROUND,
                                 change_opts->copy_mode);
       if (current != MIRROR_COPY_MODE_BACKGROUND) {
           error_setg(errp, "Expected current copy mode '%s', got '%s'",
                      MirrorCopyMode_str(MIRROR_COPY_MODE_BACKGROUND),
                      MirrorCopyMode_str(current));
       }

Now I'm curious: what could be changing @copy_mode behind our backs
here?

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 67dd0ef038..6041e7bd8f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3071,7 +3071,7 @@
   ##
   # @BlockJobChangeOptionsMirror:
   #
   # @copy-mode: Switch to this copy mode.  Currently, only the switch
   #     from 'background' to 'write-blocking' is implemented.
   #
>  # Since: 8.2
>  ##
>  { 'struct': 'JobChangeOptionsMirror',
> -  'data': { 'copy-mode' : 'MirrorCopyMode' } }
> +  'data': { '*copy-mode' : 'MirrorCopyMode' } }
>  
>  ##
>  # @JobChangeOptions:

A member becoming optional is backward compatible.  Okay.

We may want to document "(optional since 9.1)".  We haven't done so in
the past.

The doc comment needs to spell out how absent members are handled.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux