4:02 a.m.
In commit d4bf0a9, we used num_queues for an attribute in the XML, but
the consensus is that we use camelCase for that. Since there was no
release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
still have time to change it.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
This patch applies on top of Laine's RNG tightening patch [1] and it
was proposed in that thread as well.
[1] http://www.redhat.com/archives/libvir-list/2013-April/msg01320.html
---
docs/formatdomain.html.in | 2 +-
docs/schemas/domaincommon.rng | 2 +-
src/conf/domain_conf.c | 6 +++---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0cc56d9..a5be162 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2135,7 +2135,7 @@
controller. A "scsi" controller has an optional
attribute <code>model</code>, which is one of "auto",
"buslogic",
"ibmvscsi", "lsilogic", "lsisas1068",
"lsisas1078", "virtio-scsi" or
- "vmpvscsi". The attribute <code>num_queues</code>
+ "vmpvscsi". The attribute <code>numQueues</code>
(<span class="since">1.0.5 (QEMU and KVM only)</span>)
specifies
the number of queues for the controller. For best performance, it's
recommended to specify a value matching the number of vCPUs. A "usb"
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3976b82..bcb1453 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1444,7 +1444,7 @@
</attribute>
</optional>
<optional>
- <attribute name="num_queues">
+ <attribute name="numQueues">
<ref name="unsignedInt"/>
</attribute>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1643f30..0487053 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5195,10 +5195,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
def->model = -1;
}
- if ((num_queues = virXMLPropString(node, "num_queues"))) {
+ if ((num_queues = virXMLPropString(node, "numQueues"))) {
if (virStrToLong_ui(num_queues, NULL, 10, &def->num_queues) < 0) {
virReportError(VIR_ERR_XML_ERROR,
- _("Malformed 'num_queues' value
'%s'"), num_queues);
+ _("Malformed 'numQueues' value
'%s'"), num_queues);
goto error;
}
}
@@ -13531,7 +13531,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
if (def->num_queues)
- virBufferAsprintf(buf, " num_queues='%u'",
def->num_queues);
+ virBufferAsprintf(buf, " numQueues='%u'", def->num_queues);
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 009d42d..65675b7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3574,7 +3574,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'num_queues' is only supported by virtio-scsi
controller"));
+ _("'numQueues' is only supported by virtio-scsi
controller"));
return NULL;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
index dfa9cf1..063ccf7 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml
@@ -20,7 +20,7 @@
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
- <controller type='scsi' index='0' model='virtio-scsi'
num_queues='8'/>
+ <controller type='scsi' index='0' model='virtio-scsi'
numQueues='8'/>
<memballoon model='virtio'/>
</devices>
</domain>
--
1.8.1.5
9:27 a.m.
On 04/18/2013 03:02 AM, Martin Kletzander wrote:
In commit d4bf0a9, we used num_queues for an attribute in the XML,
but
the consensus is that we use camelCase for that. Since there was no
release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
still have time to change it.
You may want to wait for DV's opinion on naming, but I wasn't aware that
we had a consensus on camelCase. In fact, '_' is currently winning,
although it's hard to say whether that is limited to older interfaces.
$ prefix='\(attribute\|element\) name=.[^'\''"]*'
$ git grep "${prefix}[A-Z]" docs/schemas/ | wc
14 42 977
$ git grep "${prefix}-" docs/schemas/ | wc
29 87 1925
$ git grep "${prefix}_" docs/schemas/ | wc
45 135 3075
But I personally don't like _, and think camelCase or - is nicer, so I
could live with this change if we get consensus.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
This patch applies on top of Laine's RNG tightening patch [1] and it
was proposed in that thread as well.
[1] http://www.redhat.com/archives/libvir-list/2013-April/msg01320.html
---
docs/formatdomain.html.in | 2 +-
docs/schemas/domaincommon.rng | 2 +-
src/conf/domain_conf.c | 6 +++---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
The patch is accurate, but I don't want to give ACK without more
agreement on what naming convention we want for new XML (we should
probably patch HACKING to enforce a style if we have consensus). You
are correct that once 1.0.5 is released it will be too late to change,
so now is the time to decide.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:43 a.m.
On 04/18/2013 04:27 PM, Eric Blake wrote:
On 04/18/2013 03:02 AM, Martin Kletzander wrote:
> In commit d4bf0a9, we used num_queues for an attribute in the XML, but
> the consensus is that we use camelCase for that. Since there was no
> release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
> still have time to change it.
You may want to wait for DV's opinion on naming, but I wasn't aware that
we had a consensus on camelCase. In fact, '_' is currently winning,
although it's hard to say whether that is limited to older interfaces.
$ prefix='\(attribute\|element\) name=.[^'\''"]*'
$ git grep "${prefix}[A-Z]" docs/schemas/ | wc
14 42 977
$ git grep "${prefix}-" docs/schemas/ | wc
29 87 1925
$ git grep "${prefix}_" docs/schemas/ | wc
45 135 3075
But I personally don't like _, and think camelCase or - is nicer, so I
could live with this change if we get consensus.
At first I mistaken '-' and '_' in the other thread, but I explained
that later on. However, this commit message didn't reflect that. I
also had the feeling that the naming was mainly restricted for
attributes, not that much for elements, but of course it makes sense to
make it global.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> This patch applies on top of Laine's RNG tightening patch [1] and it
> was proposed in that thread as well.
>
> [1] http://www.redhat.com/archives/libvir-list/2013-April/msg01320.html
> ---
> docs/formatdomain.html.in | 2 +-
> docs/schemas/domaincommon.rng | 2 +-
> src/conf/domain_conf.c | 6 +++---
> src/qemu/qemu_command.c | 2 +-
> tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
>
The patch is accurate, but I don't want to give ACK without more
agreement on what naming convention we want for new XML (we should
probably patch HACKING to enforce a style if we have consensus). You
are correct that once 1.0.5 is released it will be too late to change,
so now is the time to decide.
I should've made myself clear enough that it is understandable that this
patch is more of a proposal, but I couldn't predict that the discussion
in the other thread will grow so fast in reactions (I hoped that this
will be the place to talk about our decision).
Sorry for rushing it so much, we have enough time before 1.0.5 and I'll
wait (of course) for some final decision on how the final XML should
look like.
Thanks,
Martin
10:16 a.m.
On 04/18/2013 10:27 AM, Eric Blake wrote:
On 04/18/2013 03:02 AM, Martin Kletzander wrote:
> In commit d4bf0a9, we used num_queues for an attribute in the XML, but
> the consensus is that we use camelCase for that. Since there was no
> release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
> still have time to change it.
You may want to wait for DV's opinion on naming, but I wasn't aware that
we had a consensus on camelCase. In fact, '_' is currently winning,
although it's hard to say whether that is limited to older interfaces.
$ prefix='\(attribute\|element\) name=.[^'\''"]*'
$ git grep "${prefix}[A-Z]" docs/schemas/ | wc
14 42 977
$ git grep "${prefix}-" docs/schemas/ | wc
29 87 1925
$ git grep "${prefix}_" docs/schemas/ | wc
45 135 3075
But I personally don't like _, and think camelCase or - is nicer, so I
could live with this change if we get consensus.
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> This patch applies on top of Laine's RNG tightening patch [1] and it
> was proposed in that thread as well.
>
> [1] http://www.redhat.com/archives/libvir-list/2013-April/msg01320.html
> ---
> docs/formatdomain.html.in | 2 +-
> docs/schemas/domaincommon.rng | 2 +-
> src/conf/domain_conf.c | 6 +++---
> src/qemu/qemu_command.c | 2 +-
> tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
>
The patch is accurate, but I don't want to give ACK without more
agreement on what naming convention we want for new XML (we should
probably patch HACKING to enforce a style if we have consensus).
I would agree with that - I've brought this up a couple of times, but
always am a bit uncomfortable of being possibly perceived as some sort
of obsessive-compulsive person, since it really is an aesthetic choice
and there are no written rules to back it up. (and actually I personally
have no preference).
You
are correct that once 1.0.5 is released it will be too late to change,
so now is the time to decide.
I'm now also wondering if numQueues should go in a <driver> subelement,
since it is a tuning parameter specific to the backend of one particular
scsi controller model. Similar attributes for the virtio <interface> and
[some disk model I don't feel like looking up right now] <disk> have
also been put in <driver>, and as I understand there will be a similar
attribute added to <interface> (only for virtio with vhost-net
enabled?), and it will most likely fit best in the <driver> subelement.
5:58 a.m.
On Thu, Apr 18, 2013 at 08:27:56AM -0600, Eric Blake wrote:
On 04/18/2013 03:02 AM, Martin Kletzander wrote:
> In commit d4bf0a9, we used num_queues for an attribute in the XML, but
> the consensus is that we use camelCase for that. Since there was no
> release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
> still have time to change it.
You may want to wait for DV's opinion on naming, but I wasn't aware that
we had a consensus on camelCase. In fact, '_' is currently winning,
although it's hard to say whether that is limited to older interfaces.
Hum, I don't think I ever suggested such rules at the XML level.
We have that convention for code, as eric pointed out it is not as clear
for XML (and unfortunately we can't hamonize at this point without
quite a bit of churn - we would have to support the old syntax forever
anyway).
I don't have a strong opinion on _ vs. camelcase for markup,
I guess the exact same arguments can be raised pros/cons on visibility
and existing use :-\
Seems Eric is rather adverse to _ which was looking like the winner,
I dislike '-' myself as it is used for comments, and very uncommon
in identifiers in XML (well it's barred from being in an identifier
in most language).
If Eric really can't stand '_' then maybe the simplest is really
to start standardizing on camelCase,
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
9:13 a.m.
On 04/22/2013 06:58 AM, Daniel Veillard wrote:
On Thu, Apr 18, 2013 at 08:27:56AM -0600, Eric Blake wrote:
> On 04/18/2013 03:02 AM, Martin Kletzander wrote:
>> In commit d4bf0a9, we used num_queues for an attribute in the XML, but
>> the consensus is that we use camelCase for that. Since there was no
>> release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we
>> still have time to change it.
> You may want to wait for DV's opinion on naming, but I wasn't aware that
> we had a consensus on camelCase. In fact, '_' is currently winning,
> although it's hard to say whether that is limited to older interfaces.
Hum, I don't think I ever suggested such rules at the XML level.
We have that convention for code, as eric pointed out it is not as clear
for XML (and unfortunately we can't hamonize at this point without
quite a bit of churn - we would have to support the old syntax forever
anyway).
Yeah, I don't think there's anything to gain (and lots to lose!) from
changing attributes that have already been in a release. And I think I
first heard about dislike of underscore in XML from danpb (and
incorrectly extrapolated that to you:
https://www.redhat.com/archives/libvir-list/2013-April/msg01359.html).
Of course in this case, we *could* avoid the camelCase vs underscore
entirely by just naming the attribute "queues" instead of numQueues or
num_queues (the fact that the attribute is a number makes it pretty
obvious that it is the "number of queues").
Since I first noticed the name, I'm actually more concerned that the
attribute is in the toplevel of <controller> rather than in a <driver>
subelement. It's true that this would be the first <driver> item under
<controller>, but more backend-specific tuning attributes may follow,
and it would be good to have them all collected together in the same
place. Since I've already brought this up in another thread (same
message as referenced above), I'll drop this if there's no response, but
does anyone else have an opinion about that?
I don't have a strong opinion on _ vs. camelcase for markup,
I guess the exact same arguments can be raised pros/cons on visibility
and existing use :-\
Seems Eric is rather adverse to _ which was looking like the winner,
I dislike '-' myself as it is used for comments, and very uncommon
in identifiers in XML (well it's barred from being in an identifier
in most language).
If Eric really can't stand '_' then maybe the simplest is really
to start standardizing on camelCase,
I don't have a preference either, but think that we should (as much as
possible) try to stick with one or the other, at least for new
attributes. A decision followed by a written rule in HACKING would be a
good way to make that happen.
9:43 a.m.
On 04/22/2013 10:13 AM, Laine Stump wrote:
Of course in this case, we *could* avoid the camelCase vs underscore
entirely by just naming the attribute "queues" instead of numQueues or
num_queues (the fact that the attribute is a number makes it pretty
obvious that it is the "number of queues").
And I notice that the patches for supporting multiple queues in
interfaces uses <driver queues='n'/>, so consistency would vote in favor
of using the same thing for <controller>.
10:19 a.m.
On 22/04/13 22:43, Laine Stump wrote:
On 04/22/2013 10:13 AM, Laine Stump wrote:
> Of course in this case, we *could* avoid the camelCase vs underscore
> entirely by just naming the attribute "queues" instead of numQueues or
> num_queues (the fact that the attribute is a number makes it pretty
> obvious that it is the "number of queues").
And I notice that the patches for supporting multiple queues in
interfaces uses <driver queues='n'/>, so consistency would vote in favor
of using the same thing for <controller>.
I like the 'queues', as it avoids the question whether to use underscore
or camelCase.. :-) So I don't mind changing the existing 'num_queues'
into 'queues'.
Osier
11:58 p.m.
On Mon, Apr 22, 2013 at 10:43:35AM -0400, Laine Stump wrote:
On 04/22/2013 10:13 AM, Laine Stump wrote:
> Of course in this case, we *could* avoid the camelCase vs underscore
> entirely by just naming the attribute "queues" instead of numQueues or
> num_queues (the fact that the attribute is a number makes it pretty
> obvious that it is the "number of queues").
And I notice that the patches for supporting multiple queues in
interfaces uses <driver queues='n'/>, so consistency would vote in favor
of using the same thing for <controller>.
ACK, consistency is important, it could have worked by applying an
uniform rule, but we didn't, so let's try to be consistent by naming
similary for similar purpose.
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
6:32 a.m.
On 04/22/2013 04:43 PM, Laine Stump wrote:
On 04/22/2013 10:13 AM, Laine Stump wrote:
> Of course in this case, we *could* avoid the camelCase vs underscore
> entirely by just naming the attribute "queues" instead of numQueues or
> num_queues (the fact that the attribute is a number makes it pretty
> obvious that it is the "number of queues").
And I notice that the patches for supporting multiple queues in
interfaces uses <driver queues='n'/>, so consistency would vote in favor
of using the same thing for <controller>.
I'm not the '+1' kind of guy, but this makes way more sense in the
driver element and I think we would definitely make a use of that in the
future, even though it increases the complexity of XML. Since there's
still time, should I try to change it to <driver queues... or is
somebody else against (or already did) that? (Last time I wrongly
assumed that sending the patch would speed up solving the question, so
I'm rather asking now).
Martin
6:44 p.m.
On 04/23/2013 05:32 AM, Martin Kletzander wrote:
On 04/22/2013 04:43 PM, Laine Stump wrote:
> On 04/22/2013 10:13 AM, Laine Stump wrote:
>> Of course in this case, we *could* avoid the camelCase vs underscore
>> entirely by just naming the attribute "queues" instead of numQueues or
>> num_queues (the fact that the attribute is a number makes it pretty
>> obvious that it is the "number of queues").
>
> And I notice that the patches for supporting multiple queues in
> interfaces uses <driver queues='n'/>, so consistency would vote in
favor
> of using the same thing for <controller>.
>
I'm not the '+1' kind of guy, but this makes way more sense in the
driver element and I think we would definitely make a use of that in the
future, even though it increases the complexity of XML. Since there's
still time, should I try to change it to <driver queues... or is
somebody else against (or already did) that? (Last time I wrongly
assumed that sending the patch would speed up solving the question, so
I'm rather asking now).
It still hasn't been done. Martin, can you jump in and do it? Laine,
speak up now if you were doing it as part of your vfio <driver> work,
although it didn't look like that to me.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:25 p.m.
On 04/23/2013 07:44 PM, Eric Blake wrote:
On 04/23/2013 05:32 AM, Martin Kletzander wrote:
> On 04/22/2013 04:43 PM, Laine Stump wrote:
>> On 04/22/2013 10:13 AM, Laine Stump wrote:
>>> Of course in this case, we *could* avoid the camelCase vs underscore
>>> entirely by just naming the attribute "queues" instead of numQueues
or
>>> num_queues (the fact that the attribute is a number makes it pretty
>>> obvious that it is the "number of queues").
>> And I notice that the patches for supporting multiple queues in
>> interfaces uses <driver queues='n'/>, so consistency would vote in
favor
>> of using the same thing for <controller>.
>>
> I'm not the '+1' kind of guy, but this makes way more sense in the
> driver element and I think we would definitely make a use of that in the
> future, even though it increases the complexity of XML. Since there's
> still time, should I try to change it to <driver queues... or is
> somebody else against (or already did) that? (Last time I wrongly
> assumed that sending the patch would speed up solving the question, so
> I'm rather asking now).
It still hasn't been done. Martin, can you jump in and do it? Laine,
speak up now if you were doing it as part of your vfio <driver> work,
although it didn't look like that to me.
I thought Osier had said that he would make the change...
Yep, he says that here:
https://www.redhat.com/archives/libvir-list/2013-April/msg01565.html
11:14 p.m.
On 24/04/13 11:25, Laine Stump wrote:
On 04/23/2013 07:44 PM, Eric Blake wrote:
> On 04/23/2013 05:32 AM, Martin Kletzander wrote:
>> On 04/22/2013 04:43 PM, Laine Stump wrote:
>>> On 04/22/2013 10:13 AM, Laine Stump wrote:
>>>> Of course in this case, we *could* avoid the camelCase vs underscore
>>>> entirely by just naming the attribute "queues" instead of
numQueues or
>>>> num_queues (the fact that the attribute is a number makes it pretty
>>>> obvious that it is the "number of queues").
>>> And I notice that the patches for supporting multiple queues in
>>> interfaces uses <driver queues='n'/>, so consistency would vote
in favor
>>> of using the same thing for <controller>.
>>>
>> I'm not the '+1' kind of guy, but this makes way more sense in the
>> driver element and I think we would definitely make a use of that in the
>> future, even though it increases the complexity of XML. Since there's
>> still time, should I try to change it to <driver queues... or is
>> somebody else against (or already did) that? (Last time I wrongly
>> assumed that sending the patch would speed up solving the question, so
>> I'm rather asking now).
> It still hasn't been done. Martin, can you jump in and do it? Laine,
> speak up now if you were doing it as part of your vfio <driver> work,
> although it didn't look like that to me.
I thought Osier had said that he would make the change...
Yep, he says that here:
https://www.redhat.com/archives/libvir-list/2013-April/msg01565.html
Yes, I planned to do it yesterday, but time occupied, I'm doing it.
Osier
4230
days inactive
4236
days old
12 comments
5 participants
participants (5)
-
Daniel Veillard -
Eric Blake -
Laine Stump -
Martin Kletzander -
Osier Yang