Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Feb 05, 2021 at 07:40:36AM -0600, Eric Blake wrote:
> On 2/5/21 5:13 AM, Daniel P. Berrangé wrote:
> > On Thu, Feb 04, 2021 at 01:07:07PM -0600, Eric Blake wrote:
> >> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
> >> that is ambiguous between a hex digit and the extremely large exibyte
> >> suffix, as well as a 'B' suffix for bytes.  In practice, people using
> >> hex inputs are specifying values in bytes (and would have written
> >> 0x2000000, or possibly relied on default_suffix in the case of
> >> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
> >> sense for inputs in decimal (where the user would write 32M).  But
> >> rather than outright dropping support for hex-with-suffix, let's
> >> follow our deprecation policy.  Sadly, since qemu_strtosz() does not
> >> have an Err** parameter, we pollute to stderr.
> > 
> > Err** is only appropriate for errors, not warnings, so this statement
> > can be removed.
> 
> The point of the comment was that we have no other mechanism for
> reporting the deprecation other than polluting to stderr.  And if we
> ever remove the support, we'll either have to silently reject input that
> we used to accept, or plumb through Err** handling to allow better
> reporting of WHY we are rejecting input.  But I didn't feel like adding
> Err** support now.

Yeah, I guess what I meant was that warning on stderr is the expected
standard practice for deprecations. We only need to worry about other
thing once the deprecation turns into a hard error later.

> 
> >> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char **end,
> >>      c = *endptr;
> >>      mul = suffix_mul(c, unit);
> >>      if (mul > 0) {
> >> +        if (hex) {
> >> +            fprintf(stderr, "Using a multiplier suffix on hex numbers "
> >> +                    "is deprecated: %s\n", nptr);
> > 
> > warn_report(), since IIUC, that'll get into HMP response correctly.
> 
> Yes, that sounds better.  I'll use that in v2, as well as tweaking the
> commit message.
> 
> > 
> >> +        }
> >>          endptr++;
> >>      } else {
> >>          mul = suffix_mul(default_suffix, unit);
> > 
> > Regards,
> > Daniel
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|