block/qcow2.h | 56 +++++++++++++++++++++++++++++++++++++++++++- include/qemu/units.h | 55 ------------------------------------------- 2 files changed, 55 insertions(+), 56 deletions(-)
This definitions are QCow2 specific, there is no need to expose them
in the global namespace.
This partially reverts commit 540b8492618eb.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
block/qcow2.h | 56 +++++++++++++++++++++++++++++++++++++++++++-
include/qemu/units.h | 55 -------------------------------------------
2 files changed, 55 insertions(+), 56 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 29c98d87a0..74d200c8cb 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -27,12 +27,66 @@
#include "crypto/block.h"
#include "qemu/coroutine.h"
-#include "qemu/units.h"
//#define DEBUG_ALLOC
//#define DEBUG_ALLOC2
//#define DEBUG_EXT
+#define S_1KiB 1024
+#define S_2KiB 2048
+#define S_4KiB 4096
+#define S_8KiB 8192
+#define S_16KiB 16384
+#define S_32KiB 32768
+#define S_64KiB 65536
+#define S_128KiB 131072
+#define S_256KiB 262144
+#define S_512KiB 524288
+#define S_1MiB 1048576
+#define S_2MiB 2097152
+#define S_4MiB 4194304
+#define S_8MiB 8388608
+#define S_16MiB 16777216
+#define S_32MiB 33554432
+#define S_64MiB 67108864
+#define S_128MiB 134217728
+#define S_256MiB 268435456
+#define S_512MiB 536870912
+#define S_1GiB 1073741824
+#define S_2GiB 2147483648
+#define S_4GiB 4294967296
+#define S_8GiB 8589934592
+#define S_16GiB 17179869184
+#define S_32GiB 34359738368
+#define S_64GiB 68719476736
+#define S_128GiB 137438953472
+#define S_256GiB 274877906944
+#define S_512GiB 549755813888
+#define S_1TiB 1099511627776
+#define S_2TiB 2199023255552
+#define S_4TiB 4398046511104
+#define S_8TiB 8796093022208
+#define S_16TiB 17592186044416
+#define S_32TiB 35184372088832
+#define S_64TiB 70368744177664
+#define S_128TiB 140737488355328
+#define S_256TiB 281474976710656
+#define S_512TiB 562949953421312
+#define S_1PiB 1125899906842624
+#define S_2PiB 2251799813685248
+#define S_4PiB 4503599627370496
+#define S_8PiB 9007199254740992
+#define S_16PiB 18014398509481984
+#define S_32PiB 36028797018963968
+#define S_64PiB 72057594037927936
+#define S_128PiB 144115188075855872
+#define S_256PiB 288230376151711744
+#define S_512PiB 576460752303423488
+#define S_1EiB 1152921504606846976
+#define S_2EiB 2305843009213693952
+#define S_4EiB 4611686018427387904
+#define S_8EiB 9223372036854775808
+
#define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
#define QCOW_CRYPT_NONE 0
diff --git a/include/qemu/units.h b/include/qemu/units.h
index 68a7758650..692db3fbb2 100644
--- a/include/qemu/units.h
+++ b/include/qemu/units.h
@@ -17,59 +17,4 @@
#define PiB (INT64_C(1) << 50)
#define EiB (INT64_C(1) << 60)
-#define S_1KiB 1024
-#define S_2KiB 2048
-#define S_4KiB 4096
-#define S_8KiB 8192
-#define S_16KiB 16384
-#define S_32KiB 32768
-#define S_64KiB 65536
-#define S_128KiB 131072
-#define S_256KiB 262144
-#define S_512KiB 524288
-#define S_1MiB 1048576
-#define S_2MiB 2097152
-#define S_4MiB 4194304
-#define S_8MiB 8388608
-#define S_16MiB 16777216
-#define S_32MiB 33554432
-#define S_64MiB 67108864
-#define S_128MiB 134217728
-#define S_256MiB 268435456
-#define S_512MiB 536870912
-#define S_1GiB 1073741824
-#define S_2GiB 2147483648
-#define S_4GiB 4294967296
-#define S_8GiB 8589934592
-#define S_16GiB 17179869184
-#define S_32GiB 34359738368
-#define S_64GiB 68719476736
-#define S_128GiB 137438953472
-#define S_256GiB 274877906944
-#define S_512GiB 549755813888
-#define S_1TiB 1099511627776
-#define S_2TiB 2199023255552
-#define S_4TiB 4398046511104
-#define S_8TiB 8796093022208
-#define S_16TiB 17592186044416
-#define S_32TiB 35184372088832
-#define S_64TiB 70368744177664
-#define S_128TiB 140737488355328
-#define S_256TiB 281474976710656
-#define S_512TiB 562949953421312
-#define S_1PiB 1125899906842624
-#define S_2PiB 2251799813685248
-#define S_4PiB 4503599627370496
-#define S_8PiB 9007199254740992
-#define S_16PiB 18014398509481984
-#define S_32PiB 36028797018963968
-#define S_64PiB 72057594037927936
-#define S_128PiB 144115188075855872
-#define S_256PiB 288230376151711744
-#define S_512PiB 576460752303423488
-#define S_1EiB 1152921504606846976
-#define S_2EiB 2305843009213693952
-#define S_4EiB 4611686018427387904
-#define S_8EiB 9223372036854775808
-
#endif
--
2.17.2
Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: > This definitions are QCow2 specific, there is no need to expose them > in the global namespace. > > This partially reverts commit 540b8492618eb. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> If we don't want this globally, I think we also don't want it in qcow2. Or at least reduce it to only those constants that qcow2 actually uses. Kevin
Hi Kevin, On 2/11/18 12:07, Kevin Wolf wrote: > Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: >> This definitions are QCow2 specific, there is no need to expose them >> in the global namespace. >> >> This partially reverts commit 540b8492618eb. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > If we don't want this globally, I think we also don't want it in qcow2. I only see this definitions used by block/qcow2.h (b6a95c6d1007). Per 540b8492618eb description "This is needed when a size has to be stringified" but I can't find other code requiring these definitions in the codebase. > Or at least reduce it to only those constants that qcow2 actually uses. Fine by me, I'll let Leonid opine first. Regards, Phil.
Am 02.11.2018 um 13:37 hat Philippe Mathieu-Daudé geschrieben: > Hi Kevin, > > On 2/11/18 12:07, Kevin Wolf wrote: > > Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: > > > This definitions are QCow2 specific, there is no need to expose them > > > in the global namespace. > > > > > > This partially reverts commit 540b8492618eb. > > > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > > If we don't want this globally, I think we also don't want it in qcow2. > > I only see this definitions used by block/qcow2.h (b6a95c6d1007). > > Per 540b8492618eb description "This is needed when a size has to be > stringified" but I can't find other code requiring these definitions in the > codebase. I guess the real question is: Is qcow2 the only place that needs stringification of sizes? The only value where this actually seems to be used in qcow2 is for DEFAULT_CLUSTER_SIZE, as the default value for QemuOpts. Other drivers still use plain numbers, but this is less readable. Then there is VDI which uses (1 * MiB), but that is compiled out and if you enable it, it breaks. So it needs the same fix. Are block drivers the only places where we stringify a size? I imagine some device models might use something like it, too? I don't mind too much which solution we end up using, but I'd prefer it to be universal. Kevin
On 11/2/18 9:10 AM, Kevin Wolf wrote: > Am 02.11.2018 um 13:37 hat Philippe Mathieu-Daudé geschrieben: >> Hi Kevin, >> >> On 2/11/18 12:07, Kevin Wolf wrote: >>> Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: >>>> This definitions are QCow2 specific, there is no need to expose them >>>> in the global namespace. >>>> >>>> This partially reverts commit 540b8492618eb. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> >>> If we don't want this globally, I think we also don't want it in qcow2. Agreed. I didn't want it in the first place, arguing that if we want stringification of defaults, it would be better to have a runtime function do that, rather than adding a set of near-duplicate macro names. >> >> I only see this definitions used by block/qcow2.h (b6a95c6d1007). >> >> Per 540b8492618eb description "This is needed when a size has to be >> stringified" but I can't find other code requiring these definitions in the >> codebase. > > I guess the real question is: Is qcow2 the only place that needs > stringification of sizes? Probably not. It seems like stringifying a default value is a common desire. > > The only value where this actually seems to be used in qcow2 is for > DEFAULT_CLUSTER_SIZE, as the default value for QemuOpts. Other drivers > still use plain numbers, but this is less readable. > > Then there is VDI which uses (1 * MiB), but that is compiled out and if > you enable it, it breaks. So it needs the same fix. > > Are block drivers the only places where we stringify a size? I imagine > some device models might use something like it, too? Indeed, I would prefer a patch that makes it possible for QemuOpts to pretty-print a default value using a generic runtime stringifier, rather than keeping these S_ macros around. > > I don't mind too much which solution we end up using, but I'd prefer it > to be universal. Agree. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Am 02.11.2018 um 15:52 hat Eric Blake geschrieben: > On 11/2/18 9:10 AM, Kevin Wolf wrote: > > Am 02.11.2018 um 13:37 hat Philippe Mathieu-Daudé geschrieben: > > > Hi Kevin, > > > > > > On 2/11/18 12:07, Kevin Wolf wrote: > > > > Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: > > > > > This definitions are QCow2 specific, there is no need to expose them > > > > > in the global namespace. > > > > > > > > > > This partially reverts commit 540b8492618eb. > > > > > > > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > > > > > > If we don't want this globally, I think we also don't want it in qcow2. > > Agreed. I didn't want it in the first place, arguing that if we want > stringification of defaults, it would be better to have a runtime function > do that, rather than adding a set of near-duplicate macro names. > > > > > > > I only see this definitions used by block/qcow2.h (b6a95c6d1007). > > > > > > Per 540b8492618eb description "This is needed when a size has to be > > > stringified" but I can't find other code requiring these definitions in the > > > codebase. > > > > I guess the real question is: Is qcow2 the only place that needs > > stringification of sizes? > > Probably not. It seems like stringifying a default value is a common desire. > > > > > The only value where this actually seems to be used in qcow2 is for > > DEFAULT_CLUSTER_SIZE, as the default value for QemuOpts. Other drivers > > still use plain numbers, but this is less readable. > > > > Then there is VDI which uses (1 * MiB), but that is compiled out and if > > you enable it, it breaks. So it needs the same fix. > > > > Are block drivers the only places where we stringify a size? I imagine > > some device models might use something like it, too? > > Indeed, I would prefer a patch that makes it possible for QemuOpts to > pretty-print a default value using a generic runtime stringifier, rather > than keeping these S_ macros around. The thing is just, QemuOpts is completetly string based. The default value field is const char*. Either we get rid of QemuOpts and switch everything to QAPI (nice thought, but a little unrealistic in the short term), or we add ways to add non-string values to QemuOpts (would require significant development on a piece of code we want to get rid of in the long term), or you keep doing stringification at build time (which I believe is the only reasonable choice at the moment). Kevin
Hi, On 11/2/18 5:28 PM, Kevin Wolf wrote: > Am 02.11.2018 um 15:52 hat Eric Blake geschrieben: >> On 11/2/18 9:10 AM, Kevin Wolf wrote: >>> Am 02.11.2018 um 13:37 hat Philippe Mathieu-Daudé geschrieben: >>>> Hi Kevin, >>>> >>>> On 2/11/18 12:07, Kevin Wolf wrote: >>>>> Am 02.11.2018 um 09:58 hat Philippe Mathieu-Daudé geschrieben: >>>>>> This definitions are QCow2 specific, there is no need to expose them >>>>>> in the global namespace. These are not QCOW2 specific. I wrote these for convenience in QCOW2, but there are many other places where these can be used (many pre-defined sizes are powers of two), and there are few places where they must replace the current notation, like in block/vdi.c with DEFAULT_CLUSTER_SIZE (unless an explicit value in bytes will be defined instead). >> >> Agreed. I didn't want it in the first place, arguing that if we want >> stringification of defaults, it would be better to have a runtime function >> do that, rather than adding a set of near-duplicate macro names. A runtime function will not help here, as these are used in compile time. These result in strings that are actually compiled into the binaries. >>> >>> Then there is VDI which uses (1 * MiB), but that is compiled out and if >>> you enable it, it breaks. So it needs the same fix. Yeah, I need to fix that as promised. Will do shortly. :) Leonid.
On 11/2/18 7:29 PM, Leonid Bloch wrote: >>> Agreed. I didn't want it in the first place, arguing that if we want >>> stringification of defaults, it would be better to have a runtime function >>> do that, rather than adding a set of near-duplicate macro names. > > A runtime function will not help here, as these are used in compile > time. These result in strings that are actually compiled into the binaries. Well, my point is that right now, QemuOpts outputs a hard-coded string (with no alternative), which does mean that things are compiled in. Is it worth exploring an enhancement to QemuOpts that lets it decide between either a const char * hardcoded string, or a runtime formatter callback function, and convert all existing hardcoded strings with awkward contents to instead use a new runtime formatter? Or is that just putting lipstick on a pig, since we are already trying to move away from QemuOpts into a more structured command line introspection? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.