On Fri, May 23, 2025 at 02:03:14PM +0300, Andrey Drobyshev wrote:
> >> I agree with Markus that this setting value different from what's been
> >> directly requested by user shouldn't go silent.  Having some kind of
> >> warning at the very least would be nice.
> > 
> > Okay, I'll make sure to warn if it exceeds the compile-time max.

Rather, refuse the QMP command with an error.

> > 
> >>
> >>>      ret = 0;
> >>>
> >>> @@ -1949,6 +1964,15 @@ static int nbd_open(BlockDriverState *bs, QDict 
> >>> *options, int flags,
> >>>
> >>>      nbd_client_connection_enable_retry(s->conn);
> >>>
> >>> +    /*
> >>> +     * We set s->multi_conn in nbd_process_options above, but now that
> >>> +     * we have connected if the server doesn't advertise that it is
> >>> +     * safe for multi-conn, force it to 1.
> >>> +     */
> >>> +    if (!(s->info.flags & NBD_FLAG_CAN_MULTI_CONN)) {
> >>> +        s->multi_conn = 1;
> >>> +    }
> >>
> >> Same here.
> > 
> > Here, I disagree.  But that's where better naming comes into play: if
> > there is 'max-' in the name, then the user should not be surprised if
> > they don't actually achieve the max (because the server lacked
> > support).  On the other hand, I could see how you might want to know
> > if you have a mismatched setup ("I think the server SHOULD be
> > supporting multi-conn, so I request multiple clients, and I want to be
> > informed if my expectations were not met because then I know to go
> > reconfigure the server").  Thoughts?
> > 
> 
> Doesn't the "max-" part suggest that there might be anything in between
> [1..N]?  When in reality it's either of {1, min(N, MAX_MULTI_CONN)}.
> But you're right, my initial argument was that this mismatch shouldn't
> go unnoticed as well.  Although I agree that it's part of the expected
> behavior and therefore might not deserve a warning.  Maybe smth like
> info_report() will do?  We might even print it unconditionally, so that
> there's always a way to tell the actual number of connections chosen.
> Somewhat similar to what Richard pointed out at in nbdcopy.

Warnings in QMP are difficult.  It's easy to fail a command, but hard
to print non-fatal messages.  However, I'm not opposed to having a way
to use a QMP query-* command as a followup for the user who is curious
on how many connections a given block export is using (either we can
already query all existing block exports, or that's something that I
should add independent of the new multi-conn setting).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org


Reply via email to