Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 16:42, Anthony Liguori ha scritto: There's such thing as list support in QemuOpts. The only way QemuOptsVisitor was able to implement it was to expose QemuOpts publicly via options_int.h and rely on a implementation detail. There are fixed types supported by QemuOpts. It just so happens that whenever qemu_opt_set() is called, instead of replacing the last instance, the value is either prepended or appended in order to implement a replace or set-if-unset behavior. Fair enough. Nobody said the implementation is pretty. If we want to have list syntax, we need to introduce first class support for it. Here's a simple example of how to do this. If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. No, I'm primarily motivated by the UI and am assuming that ya'll are arguing based on the implementation today. Your assumption is incorrect :) Repeating keys is quite mad. Here are some examples: qemu -numa node=1,node=2,cpus=2,cpus=3 What would a user expect that that would mean. I figure you mean -numa node,nodeid=1,nodeid=2,cpus=2,cpus=3 Parameter nodeid is int-valued, therefore repeating it is either an error, or the last one wins. Matter of taste. Currently, we do the latter. Parameter cpus is list-valued, therefore the values accumulate. We already do that. Taken together, this configures node 1 to use cpus 2 and 3. What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 With respect to escaping, for string lists (the only place where this point is even relevant), we can simply support escaping. But I'd like to hear a use-case for a string list first. There is no need for the syntactic warts you so cavalierly propose. Whenever you do key=X:Y, I can do key=X,key=Y. Whatever semantics you propose for the former, I'll propose for the latter. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 17:19, Eduardo Habkost ha scritto: If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. In the meanwhile, however, it seems to me that Eduardo can use QemuOptsVisitor---which can also hide the details and provide type safety. Whatever I use to implement it, I still need to know how the command-line syntax will look like, because we need to tell libvirt developers how they should write the QEMU command-line. I don't think any syntax makes sense except cpus=A,cpus=B. How we implement it is another story. Agreed on both counts. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 18:08, Anthony Liguori ha scritto: No, no, no. This makes ':' special, which means you can't have lists of anything containing ':'. Your cure is worse than the disease. Let go of that syntactic high-fructose corn syrup, stick to what we have and works just fine, thank you. Yes, there *must* be special syntax. If we're treating something special, then we should indicate to the user that it's special. Specifically, a list of integers should look distinctly different than overriding a previously specified integer. The solution is there is no way to override a previously specified key. Something like -device virtio-scsi-pci,num_queues=1,num_queues=2 now works, let's make it an error instead. That breaks compatibility. The above may seem silly but consider: qemu -device virtio-scsi-pci,num_queues=1,id=foo \ -set device.foo.num_queues=2 This is more common than you would think primarily as a way to override options that libvirt has set either via the qemu extra args tag or a script wrapper of qemu. Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Markus Armbruster arm...@redhat.com writes: Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. With QemuOpts, this is treated as two distinct sections with anonymous ids so whatever the code is to handle NUMA options would get called twice and it's up to that code to determine how it handles the conflict. (This is admittedly irrelevant to the discussion.) Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? Regards, Anthony Liguori With respect to escaping, for string lists (the only place where this point is even relevant), we can simply support escaping. But I'd like to hear a use-case for a string list first. There is no need for the syntactic warts you so cavalierly propose. Whenever you do key=X:Y, I can do key=X,key=Y. Whatever semantics you propose for the former, I'll propose for the latter. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Markus Armbruster arm...@redhat.com writes: Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? My fundamental problem here is that we have the same syntax with different meanings depending on the context. Going back to our original example: qemu -numa node,nodeid=2,cpus=4 This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. Regards, Anthony Liguori -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 28/02/2013 14:32, Anthony Liguori ha scritto: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? For example because another kind of string list could have a colon inside, and that would in turn need escaping (in fact that's already the case for guestfwd); repeating the key is a syntax that is easily reusable (and indeed is already in use). Instead, the '-' is parsed within the NUMA code, and is completely opaque to QemuOpts. The NUMA code knows that the '-' will never need escaping, because it only deals with positive integers. A perhaps better question would have been if you're okay with making ',' be special syntax, why not ':'. And the answer is that indeed ',' already brings some problems, but likely they outweight the advantages of having say only a -set option. But adding a second escaped character is already much more debatable in my opinion. What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? What would the QemuOpts parsing code do? Do you have in mind bitmasks as a first-class QemuOpts type? If so, that would be an argument in favor of ':', but against the prototype patch you posted yesterday. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 28/02/2013 14:41, Anthony Liguori ha scritto: This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. This makes your point clear, but it sounds a bit artificial. 4 or 8 would never appear alone. You would likely have something like -numa node,nodeid=0,cpus=0,cpus=12 \ -numa node,nodeid=1,cpus=1,cpus=13 \ -numa node,nodeid=2,cpus=2,cpus=14 \ -numa node,nodeid=3,cpus=3,cpus=15 \ -numa node,nodeid=4,cpus=4,cpus=8 which would make the syntax much more obvious. Something like 4:8 would be rather unclear actually, because both numbes are even. Given -numa node,nodeid=2,cpus=4:8 out of context, I would guess that 4:8 is [4,8) where the upper-bound is excluded for some reason. Of course context would clear it up, but that also applies to cpus=foo,cpus=bar. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? My fundamental problem here is that we have the same syntax with different meanings depending on the context. Going back to our original example: qemu -numa node,nodeid=2,cpus=4 This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Root cause: the name cpus can be interpreted as number of cpus or as list of cpus. Fix (if it's worth fixing): use a better name. First one that crossed my mind: cpu. Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and I suspect a reasonable person is blissfully unaware of the fact that he can give the same key several times in a single option argument, let alone what happens when he does. And I still think we could outlaw such repetition if we cared. Besides the command line, there's also the config file. As Paolo explained, repeated key means list is established practice there. therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. Does it mean CPU 4 and 8? CPU 4 to 8? 8 CPUs starting with 4? If it's less ambiguous, then probably because it's sufficiently greek to make people reach for the manual :) Moreover, no change, thus no improvement for your original example cpus=4, which you called certainly ambigous. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. With QemuOpts, this is treated as two distinct sections with anonymous ids so whatever the code is to handle NUMA options would get called twice and it's up to that code to determine how it handles the conflict. (This is admittedly irrelevant to the discussion.) Correct. Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? Fair question! What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? Yes, and another fair question. I want common QemuOpts syntax used instead of ad hoc syntax whenever practical, because ad hoc syntax is bound to breed inconsistencies and special cases. QemuOpts was created to get us out of that pit; let's not jump back in without a really good cause. We already got lists in QemuOpts. We don't have intervals in QemuOpts. If we had, I'd certainly argue for using them here. If more uses of intervals turn up, I'll argue for putting intervals in QemuOpts. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori aligu...@us.ibm.com writes:
Paolo Bonzini pbonz...@redhat.com writes:
Il 26/02/2013 20:35, Anthony Liguori ha scritto:
See also discussion on multi-valued keys in command line option
arguments and config files in v1 thread. Hopefully we can reach a
conclusion soon, and then we'll see whether this patch is what we want.
Yeah, let's drop this patch by now. I am starting to be convinced that
cpus=A,cpus=B,cpus=C is the best approach. It is not pretty, but at
least it uses generic parser code instead of yet another ad-hoc
parser.
No, we cannot rely on this behavior. We had to do it to support
backwards compat with netdev but it should not be used anywhere else.
We chose a config format (git's) because it was a good match for
QemuOpts, and multiple-valued operations are well supported by that
config format; see git-config(1). There is no reason to consider it a
backwards-compatibility hack.
There's such thing as list support in QemuOpts. The only way
QemuOptsVisitor was able to implement it was to expose QemuOpts publicly
via options_int.h and rely on a implementation detail.
There are fixed types supported by QemuOpts. It just so happens that
whenever qemu_opt_set() is called, instead of replacing the last
instance, the value is either prepended or appended in order to
implement a replace or set-if-unset behavior.
All values are treated this way. So the above list would only be:
{
.name = cpus,
.type = QEMU_OPT_STRING
}
Which is indistinguishable from a straight string property. This means
it's impossible to introspect because the type is context-sensitive.
What's more, there is no API outside of QemuOptsVisitor that can
actually work with lists of QemuOpts values.
There is: qemu_opt_foreach()
If you relax your claim to QemuOpts API for lists sucks and needs
improvement, then we're in violent agreement.
If we want to have list syntax, we need to introduce first class support
for it. Here's a simple example of how to do this. Obviously we would
want to introduce some better error checking so we can catch if someone
tries to access a list with a non-list accessor. We also would need
list accessor functions.
But it's really not hard at all.
(Only compile tested)
From 4ee7ed36d597f0defd78baac7480ecb29e11e1c1 Mon Sep 17 00:00:00 2001
From: Anthony Liguori aligu...@us.ibm.com
Date: Wed, 27 Feb 2013 09:32:09 -0600
Subject: [PATCH] qemu-opts: support lists
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
include/qemu/option.h | 2 ++
util/qemu-option.c| 12
2 files changed, 14 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index ba197cd..e4808c3 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -41,6 +41,7 @@ enum QEMUOptionParType {
typedef struct QEMUOptionParameter {
const char *name;
enum QEMUOptionParType type;
+bool list;
union {
uint64_t n;
char* s;
@@ -95,6 +96,7 @@ enum QemuOptType {
typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
+bool list;
const char *help;
} QemuOptDesc;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 5a1d03c..6b1ae6e 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -636,6 +636,18 @@ static void opt_set(QemuOpts *opts, const char *name,
const char *value,
return;
}
+if (desc-list strchr(value, ':')) {
+gchar **tokens = g_strsplit(value, :, 0);
+int i;
+for (i = 0; tokens[i]; i++) {
+opt_set(opts, name, tokens[i], prepend, errp);
+if (error_is_set(errp)) {
+return;
+}
+}
+g_strfreev(tokens);
+}
+
opt = g_malloc0(sizeof(*opt));
opt-name = g_strdup(name);
opt-opts = opts;
No, no, no. This makes ':' special, which means you can't have lists of
anything containing ':'. Your cure is worse than the disease. Let go
of that syntactic high-fructose corn syrup, stick to what we have and
works just fine, thank you.
Then add suitable list accessor functions and error checks.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 16:42, Anthony Liguori ha scritto: There's such thing as list support in QemuOpts. The only way QemuOptsVisitor was able to implement it was to expose QemuOpts publicly via options_int.h and rely on a implementation detail. There are fixed types supported by QemuOpts. It just so happens that whenever qemu_opt_set() is called, instead of replacing the last instance, the value is either prepended or appended in order to implement a replace or set-if-unset behavior. Fair enough. Nobody said the implementation is pretty. If we want to have list syntax, we need to introduce first class support for it. Here's a simple example of how to do this. If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. No, I'm primarily motivated by the UI and am assuming that ya'll are arguing based on the implementation today. Repeating keys is quite mad. Here are some examples: qemu -numa node=1,node=2,cpus=2,cpus=3 What would a user expect that that would mean. What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I'm pretty sure you won't be able to say without looking at the source code. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 With respect to escaping, for string lists (the only place where this point is even relevant), we can simply support escaping. But I'd like to hear a use-case for a string list first. Regards, Anthony Liguori In the meanwhile, however, it seems to me that Eduardo can use QemuOptsVisitor---which can also hide the details and provide type safety. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Markus Armbruster arm...@redhat.com writes: Anthony Liguori aligu...@us.ibm.com writes: Which is indistinguishable from a straight string property. This means it's impossible to introspect because the type is context-sensitive. What's more, there is no API outside of QemuOptsVisitor that can actually work with lists of QemuOpts values. There is: qemu_opt_foreach() I'm not sure I believe that you wrote that with a straight face... ;-) opt = g_malloc0(sizeof(*opt)); opt-name = g_strdup(name); opt-opts = opts; No, no, no. This makes ':' special, which means you can't have lists of anything containing ':'. Your cure is worse than the disease. Let go of that syntactic high-fructose corn syrup, stick to what we have and works just fine, thank you. Yes, there *must* be special syntax. If we're treating something special, then we should indicate to the user that it's special. Specifically, a list of integers should look distinctly different than overriding a previously specified integer. Regards, Anthony Liguori Then add suitable list accessor functions and error checks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Eduardo Habkost ehabk...@redhat.com writes: On Wed, Feb 27, 2013 at 04:57:15PM +0100, Paolo Bonzini wrote: Il 27/02/2013 16:42, Anthony Liguori ha scritto: There's such thing as list support in QemuOpts. The only way QemuOptsVisitor was able to implement it was to expose QemuOpts publicly via options_int.h and rely on a implementation detail. There are fixed types supported by QemuOpts. It just so happens that whenever qemu_opt_set() is called, instead of replacing the last instance, the value is either prepended or appended in order to implement a replace or set-if-unset behavior. Fair enough. Nobody said the implementation is pretty. If we want to have list syntax, we need to introduce first class support for it. Here's a simple example of how to do this. If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. In the meanwhile, however, it seems to me that Eduardo can use QemuOptsVisitor---which can also hide the details and provide type safety. Whatever I use to implement it, I still need to know how the command-line syntax will look like, because we need to tell libvirt developers how they should write the QEMU command-line. Command line syntax is not committed until it appears in a release. libvirt *should not* assume any specific syntax until the 1.5 release ships. Regards, Anthony Liguori -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 27/02/2013 18:08, Anthony Liguori ha scritto: No, no, no. This makes ':' special, which means you can't have lists of anything containing ':'. Your cure is worse than the disease. Let go of that syntactic high-fructose corn syrup, stick to what we have and works just fine, thank you. Yes, there *must* be special syntax. If we're treating something special, then we should indicate to the user that it's special. Specifically, a list of integers should look distinctly different than overriding a previously specified integer. The solution is there is no way to override a previously specified key. Something like -device virtio-scsi-pci,num_queues=1,num_queues=2 now works, let's make it an error instead. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
On Wed, Feb 27, 2013 at 11:04:08AM -0600, Anthony Liguori wrote: Eduardo Habkost ehabk...@redhat.com writes: On Wed, Feb 27, 2013 at 04:57:15PM +0100, Paolo Bonzini wrote: Il 27/02/2013 16:42, Anthony Liguori ha scritto: There's such thing as list support in QemuOpts. The only way QemuOptsVisitor was able to implement it was to expose QemuOpts publicly via options_int.h and rely on a implementation detail. There are fixed types supported by QemuOpts. It just so happens that whenever qemu_opt_set() is called, instead of replacing the last instance, the value is either prepended or appended in order to implement a replace or set-if-unset behavior. Fair enough. Nobody said the implementation is pretty. If we want to have list syntax, we need to introduce first class support for it. Here's a simple example of how to do this. If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. In the meanwhile, however, it seems to me that Eduardo can use QemuOptsVisitor---which can also hide the details and provide type safety. Whatever I use to implement it, I still need to know how the command-line syntax will look like, because we need to tell libvirt developers how they should write the QEMU command-line. Command line syntax is not committed until it appears in a release. libvirt *should not* assume any specific syntax until the 1.5 release ships. I am just talking about communication with libvirt developers. Developers surely write code and test their work in progress using unreleased QEMU code, instead of waiting for an official release. Nobody suggested releasing a libvirt version that relies on an unreleased QEMU feature. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
On 02/27/2013 10:04 AM, Anthony Liguori wrote: Whatever I use to implement it, I still need to know how the command-line syntax will look like, because we need to tell libvirt developers how they should write the QEMU command-line. Command line syntax is not committed until it appears in a release. libvirt *should not* assume any specific syntax until the 1.5 release ships. Libvirt 1.0.3 will already error out on any disjoint ranges, and we are just fine waiting until qemu 1.5 is released before adding any code to support disjoint ranges in the preferred syntax. Qemu should feel free to implement it correctly, rather than trying to cater to older (broken) libvirt's abuse of comma. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 18:08, Anthony Liguori ha scritto: No, no, no. This makes ':' special, which means you can't have lists of anything containing ':'. Your cure is worse than the disease. Let go of that syntactic high-fructose corn syrup, stick to what we have and works just fine, thank you. Yes, there *must* be special syntax. If we're treating something special, then we should indicate to the user that it's special. Specifically, a list of integers should look distinctly different than overriding a previously specified integer. The solution is there is no way to override a previously specified key. Something like -device virtio-scsi-pci,num_queues=1,num_queues=2 now works, let's make it an error instead. That breaks compatibility. The above may seem silly but consider: qemu -device virtio-scsi-pci,num_queues=1,id=foo \ -set device.foo.num_queues=2 This is more common than you would think primarily as a way to override options that libvirt has set either via the qemu extra args tag or a script wrapper of qemu. Regards, Anthony Liguori Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 27/02/2013 18:38, Anthony Liguori ha scritto: The solution is there is no way to override a previously specified key. Something like -device virtio-scsi-pci,num_queues=1,num_queues=2 now works, let's make it an error instead. That breaks compatibility. The above may seem silly but consider: qemu -device virtio-scsi-pci,num_queues=1,id=foo \ -set device.foo.num_queues=2 This is more common than you would think primarily as a way to override options that libvirt has set either via the qemu extra args tag or a script wrapper of qemu. -set could first delete a pre-existing option. We could also extend -set to accept multiple values (like -set device.foo.bar=baz,device.foo.qux=quux), which is useful also to set a list option. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Eduardo Habkost ehabk...@redhat.com writes: This allows : to be used a separator between each CPU range, so the command-line may look like: -numa node,cpus=A-B:C-D Note that the following format, currently used by libvirt: -numa nodes,cpus=A-B,C-D will _not_ work, as , is the option separator for the command-line option parser, and it would require changing the -numa option parsing code to handle cpus as a special case. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- Changes v2: - Use : as separator - Document the new format See also discussion on multi-valued keys in command line option arguments and config files in v1 thread. Hopefully we can reach a conclusion soon, and then we'll see whether this patch is what we want. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote: Eduardo Habkost ehabk...@redhat.com writes: This allows : to be used a separator between each CPU range, so the command-line may look like: -numa node,cpus=A-B:C-D Note that the following format, currently used by libvirt: -numa nodes,cpus=A-B,C-D will _not_ work, as , is the option separator for the command-line option parser, and it would require changing the -numa option parsing code to handle cpus as a special case. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- Changes v2: - Use : as separator - Document the new format See also discussion on multi-valued keys in command line option arguments and config files in v1 thread. Hopefully we can reach a conclusion soon, and then we'll see whether this patch is what we want. Yeah, let's drop this patch by now. I am starting to be convinced that cpus=A,cpus=B,cpus=C is the best approach. It is not pretty, but at least it uses generic parser code instead of yet another ad-hoc parser. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Eduardo Habkost ehabk...@redhat.com writes: On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote: Eduardo Habkost ehabk...@redhat.com writes: This allows : to be used a separator between each CPU range, so the command-line may look like: -numa node,cpus=A-B:C-D Note that the following format, currently used by libvirt: -numa nodes,cpus=A-B,C-D will _not_ work, as , is the option separator for the command-line option parser, and it would require changing the -numa option parsing code to handle cpus as a special case. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- Changes v2: - Use : as separator - Document the new format See also discussion on multi-valued keys in command line option arguments and config files in v1 thread. Hopefully we can reach a conclusion soon, and then we'll see whether this patch is what we want. Yeah, let's drop this patch by now. I am starting to be convinced that cpus=A,cpus=B,cpus=C is the best approach. It is not pretty, but at least it uses generic parser code instead of yet another ad-hoc parser. No, we cannot rely on this behavior. We had to do it to support backwards compat with netdev but it should not be used anywhere else. Regards, Anthony Liguori -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
On Tue, Feb 26, 2013 at 01:35:14PM -0600, Anthony Liguori wrote: Eduardo Habkost ehabk...@redhat.com writes: On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote: Eduardo Habkost ehabk...@redhat.com writes: This allows : to be used a separator between each CPU range, so the command-line may look like: -numa node,cpus=A-B:C-D Note that the following format, currently used by libvirt: -numa nodes,cpus=A-B,C-D will _not_ work, as , is the option separator for the command-line option parser, and it would require changing the -numa option parsing code to handle cpus as a special case. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- Changes v2: - Use : as separator - Document the new format See also discussion on multi-valued keys in command line option arguments and config files in v1 thread. Hopefully we can reach a conclusion soon, and then we'll see whether this patch is what we want. Yeah, let's drop this patch by now. I am starting to be convinced that cpus=A,cpus=B,cpus=C is the best approach. It is not pretty, but at least it uses generic parser code instead of yet another ad-hoc parser. No, we cannot rely on this behavior. We had to do it to support backwards compat with netdev but it should not be used anywhere else. Why? What should be the proper and generic way to represent and parse lists, then? There are many arguments being exposed at the v1 thread. See Message-ID: 512cc6d6.4060...@redhat.com. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Paolo Bonzini pbonz...@redhat.com writes: Il 26/02/2013 20:35, Anthony Liguori ha scritto: See also discussion on multi-valued keys in command line option arguments and config files in v1 thread. Hopefully we can reach a conclusion soon, and then we'll see whether this patch is what we want. Yeah, let's drop this patch by now. I am starting to be convinced that cpus=A,cpus=B,cpus=C is the best approach. It is not pretty, but at least it uses generic parser code instead of yet another ad-hoc parser. No, we cannot rely on this behavior. We had to do it to support backwards compat with netdev but it should not be used anywhere else. We chose a config format (git's) because it was a good match for QemuOpts, and multiple-valued operations are well supported by that config format; see git-config(1). There is no reason to consider it a backwards-compatibility hack. Seconded. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
