[PATCH 2/3] utils: Deprecate hex-with-suffix sizes
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. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/system/deprecated.rst | 8 ++++++++ util/cutils.c | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 6ac757ed9fa7..c404c3926e6f 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts. +hexadecimal sizes with scaling multipliers (since 6.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Input parameters that take a size value should only use a size suffix +(such as 'k' or 'M') when the base is written in decimal, and not when +the value is hexadecimal. That is, '0x20M' is deprecated, and should +be written either as '32M' or as '0x2000000'. + QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/util/cutils.c b/util/cutils.c index 0234763bd70b..75190565cbb5 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end, int retval; const char *endptr; unsigned char c; - bool mul_required = false; + bool mul_required = false, hex = false; uint64_t val; int64_t mul; double fraction = 0.0; @@ -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); + } endptr++; } else { mul = suffix_mul(default_suffix, unit); -- 2.30.0
04.02.2021 22:07, Eric Blake wrote:
Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
What about also deprecating 'E' suffix? (just my problem of reviewing previous patch)
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.
Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/system/deprecated.rst | 8 ++++++++ util/cutils.c | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 6ac757ed9fa7..c404c3926e6f 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts.
+hexadecimal sizes with scaling multipliers (since 6.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Input parameters that take a size value should only use a size suffix +(such as 'k' or 'M') when the base is written in decimal, and not when +the value is hexadecimal. That is, '0x20M' is deprecated, and should +be written either as '32M' or as '0x2000000'. + QEMU Machine Protocol (QMP) commands ------------------------------------
diff --git a/util/cutils.c b/util/cutils.c index 0234763bd70b..75190565cbb5 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end, int retval; const char *endptr; unsigned char c; - bool mul_required = false; + bool mul_required = false, hex = false; uint64_t val; int64_t mul; double fraction = 0.0; @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char **end,
you forget to set hex to true in corresponding if(){...}
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); + } endptr++; } else { mul = suffix_mul(default_suffix, unit);
should we also deprecate hex where default_suffix is not 'B' ? -- Best regards, Vladimir
On Fri, Feb 05, 2021 at 01:25:18PM +0300, Vladimir Sementsov-Ogievskiy wrote:
04.02.2021 22:07, Eric Blake wrote:
Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
What about also deprecating 'E' suffix? (just my problem of reviewing previous patch)
Ha! What if people want to specify exabytes?! That actually works at the moment: $ nbdkit memory 7E --run 'qemu-io -f raw -c "r -v 1E 512" "$uri"' Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On 2/5/21 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
04.02.2021 22:07, Eric Blake wrote:
Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
What about also deprecating 'E' suffix? (just my problem of reviewing previous patch)
No, we want to keep '1E' as a valid way to spell 1 exabyte. That has uses in the wild (admittedly corner-case, as that is a LOT of storage). It is only '0x1E' where the use of 'E' as a hex digit has priority over 'E' as a suffix meaning exabyte.
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.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
+++ b/util/cutils.c @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end, int retval; const char *endptr; unsigned char c; - bool mul_required = false; + bool mul_required = false, hex = false; uint64_t val; int64_t mul; double fraction = 0.0; @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char **end,
you forget to set hex to true in corresponding if(){...}
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); + }
D'oh. Now I get to rerun my tests to see when the warning triggers.
endptr++; } else { mul = suffix_mul(default_suffix, unit);
should we also deprecate hex where default_suffix is not 'B' ?
That's exactly what this patch is (supposed to be) doing. If we parsed a hex number, and there was an explicit suffix at all (which is necessarily neither 'B' nor 'E', since those were already consumed while parsing the hex number), issue a warning. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
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.
Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/system/deprecated.rst | 8 ++++++++ util/cutils.c | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 6ac757ed9fa7..c404c3926e6f 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts.
+hexadecimal sizes with scaling multipliers (since 6.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Input parameters that take a size value should only use a size suffix +(such as 'k' or 'M') when the base is written in decimal, and not when +the value is hexadecimal. That is, '0x20M' is deprecated, and should +be written either as '32M' or as '0x2000000'. + QEMU Machine Protocol (QMP) commands ------------------------------------
diff --git a/util/cutils.c b/util/cutils.c index 0234763bd70b..75190565cbb5 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end, int retval; const char *endptr; unsigned char c; - bool mul_required = false; + bool mul_required = false, hex = false; uint64_t val; int64_t mul; double fraction = 0.0; @@ -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.
+ } endptr++; } else { mul = suffix_mul(default_suffix, unit);
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 :|
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
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 :|
participants (4)
-
Daniel P. Berrangé -
Eric Blake -
Richard W.M. Jones -
Vladimir Sementsov-Ogievskiy