On 4/5/19 2:16 PM, Max Reitz wrote: > The existing code to convert flag bits into strings looks a bit strange > now, and if we ever add more flags, it will look even stranger. Prevent > that from happening by making it look up the flag names in an array.
At one point, I even considered using a QAPI type and expressing things
in a way where we could add an --output=json and/or use our existing
QObject-to-human-readable formatters instead of open-coding things in
qemu-nbd. But this patch is worthwhile in the meantime.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> Looking at the diff stat I can't claim this patch really improves
> anything by much, but the current code just pains me so.
> ---
> include/block/nbd.h | 38 +++++++++++++++++++++++++------------
> qemu-nbd.c | 46 +++++++++++++++++----------------------------
> 2 files changed, 43 insertions(+), 41 deletions(-)
Reviewed-by: Eric Blake <ebl...@redhat.com>
Will apply through my NBD tree for 4.1 (no semantic change, so -rc3 is
indeed too late for taking this into 4.0)
Hmm. nbdkit uses a generated header that scrapes the definitions of
various bit values and produces automated value-to-name lookup
functions, rather than having to open-code the lookup functions. Perhaps
we could consider doing similar in qemu for even less code to maintain.
> + [NBD_FLAG_SEND_CACHE_BIT] = "cache",
> + };
> +
> printf(" size: %" PRIu64 "\n", list[i].size);
> printf(" flags: 0x%x (", list[i].flags);
> - if (list[i].flags & NBD_FLAG_READ_ONLY) {
> - printf(" readonly");
> - }
I'm also wondering if I should have added another nbd_*_lookup()
function in nbd/common.c to do this lookup (even if we still like the
array approach over open-coding, and whether or not we like the idea of
generating things from the .h, having all of the lookup functions in one
place and style makes more sense than special-casing NBD_FLAG_ lookups
to just live in qemu-nbd.c).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
