* [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
@ 2018-11-02 8:58 Philippe Mathieu-Daudé
2018-11-02 11:07 ` Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-02 8:58 UTC (permalink / raw)
To: Kevin Wolf, Max Reitz, Leonid Bloch, Alberto Garcia
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-block, Markus Armbruster
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 8:58 [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions Philippe Mathieu-Daudé
@ 2018-11-02 11:07 ` Kevin Wolf
2018-11-02 12:37 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2018-11-02 11:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Max Reitz, Leonid Bloch, Alberto Garcia, qemu-devel, qemu-block,
Markus Armbruster
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 11:07 ` Kevin Wolf
@ 2018-11-02 12:37 ` Philippe Mathieu-Daudé
2018-11-02 14:10 ` Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-02 12:37 UTC (permalink / raw)
To: Kevin Wolf
Cc: Max Reitz, Leonid Bloch, Alberto Garcia, qemu-devel, qemu-block,
Markus Armbruster
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 12:37 ` Philippe Mathieu-Daudé
@ 2018-11-02 14:10 ` Kevin Wolf
2018-11-02 14:52 ` Eric Blake
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2018-11-02 14:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Max Reitz, Leonid Bloch, Alberto Garcia, qemu-devel, qemu-block,
Markus Armbruster
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 14:10 ` Kevin Wolf
@ 2018-11-02 14:52 ` Eric Blake
2018-11-02 15:28 ` Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2018-11-02 14:52 UTC (permalink / raw)
To: Kevin Wolf, Philippe Mathieu-Daudé
Cc: Alberto Garcia, qemu-block, Markus Armbruster, qemu-devel,
Leonid Bloch, Max Reitz
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 14:52 ` Eric Blake
@ 2018-11-02 15:28 ` Kevin Wolf
2018-11-03 0:29 ` Leonid Bloch
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2018-11-02 15:28 UTC (permalink / raw)
To: Eric Blake
Cc: Philippe Mathieu-Daudé,
Alberto Garcia, qemu-block, Markus Armbruster, qemu-devel,
Leonid Bloch, Max Reitz
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-02 15:28 ` Kevin Wolf
@ 2018-11-03 0:29 ` Leonid Bloch
2018-11-05 15:42 ` Eric Blake
0 siblings, 1 reply; 8+ messages in thread
From: Leonid Bloch @ 2018-11-03 0:29 UTC (permalink / raw)
To: Kevin Wolf, Eric Blake
Cc: Philippe Mathieu-Daudé,
Alberto Garcia, qemu-block, Markus Armbruster, qemu-devel,
Max Reitz
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions
2018-11-03 0:29 ` Leonid Bloch
@ 2018-11-05 15:42 ` Eric Blake
0 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2018-11-05 15:42 UTC (permalink / raw)
To: Leonid Bloch, Kevin Wolf
Cc: Philippe Mathieu-Daudé,
Alberto Garcia, qemu-block, Markus Armbruster, qemu-devel,
Max Reitz
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-05 15:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 8:58 [Qemu-devel] [PATCH] qemu/units: Move out QCow2 specific definitions Philippe Mathieu-Daudé
2018-11-02 11:07 ` Kevin Wolf
2018-11-02 12:37 ` Philippe Mathieu-Daudé
2018-11-02 14:10 ` Kevin Wolf
2018-11-02 14:52 ` Eric Blake
2018-11-02 15:28 ` Kevin Wolf
2018-11-03 0:29 ` Leonid Bloch
2018-11-05 15:42 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.