qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: vsementsov@virtuozzo.com, qemu-block@nongnu.org,
	rjones@redhat.com,
	"reviewer:Incompatible changes" <libvir-list@redhat.com>,
	tao3.xu@intel.com, armbru@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
Date: Fri, 5 Feb 2021 07:40:36 -0600	[thread overview]
Message-ID: <f08f11a9-f76c-4473-a770-9939f28c7373@redhat.com> (raw)
In-Reply-To: <20210205111302.GF908621@redhat.com>

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.

>> @@ -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



  reply	other threads:[~2021-02-05 13:42 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-04 19:07 [PATCH 0/3] Improve do_strtosz precision Eric Blake
2021-02-04 19:07 ` [PATCH 1/3] utils: Improve qemu_strtosz() to have 64 bits of precision Eric Blake
2021-02-04 20:12   ` Eric Blake
2021-02-05 10:06     ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:18       ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:06       ` Eric Blake
2021-02-05 14:10         ` Daniel P. Berrangé
2021-02-05 10:07   ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:12     ` Eric Blake
2021-02-05 10:28   ` Richard W.M. Jones
2021-02-05 14:15     ` Eric Blake
2021-02-05 11:02   ` Daniel P. Berrangé
2021-02-05 14:27     ` Eric Blake
2021-02-05 14:36       ` Daniel P. Berrangé
2021-02-05 11:34   ` Daniel P. Berrangé
2021-02-05 14:36     ` Eric Blake
2021-02-04 19:07 ` [PATCH 2/3] utils: Deprecate hex-with-suffix sizes Eric Blake
2021-02-05 10:25   ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:31     ` Richard W.M. Jones
2021-02-05 13:38     ` Eric Blake
2021-02-05 11:13   ` Daniel P. Berrangé
2021-02-05 13:40     ` Eric Blake [this message]
2021-02-05 14:02       ` Daniel P. Berrangé
2021-02-04 19:07 ` [PATCH 3/3] utils: Deprecate inexact fractional suffix sizes Eric Blake
2021-02-04 20:02   ` Eric Blake
2021-02-05 10:34   ` Richard W.M. Jones
2021-02-05 14:19     ` Eric Blake
2021-02-05 10:38   ` Vladimir Sementsov-Ogievskiy
2021-02-05 11:10   ` Daniel P. Berrangé
2021-02-05 14:28     ` Eric Blake
2021-02-05 14:40       ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f08f11a9-f76c-4473-a770-9939f28c7373@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=tao3.xu@intel.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).