Mailing List Archive: Bug in xm config parsing code

Hi,

I think I've found a bug in xm/create.py:

http://lists.xensource.com/archives/html/xen-changelog/2009-11/msg00132.html

introduces the "maxvcpus" parameter. It states that

"this is patch to add maxvcpus support to xen xm command. It's using
vcpu_avail bitmask and sets the number of vcpus to maxvcpus if
present. If it's not present, old behavior is preserved."

The last part is untrue as far as I understand. If maxvcpus is unset,
vcpu_avail is not read at all. The patch removes it from the parameter
list given to add_conf without adding it in another place. As a result
vcpu_avail remains unset and is filled with the default value of 255 (?)
later.

This patch to fixes the issue for me:

--- create.py 2010-09-08 15:52:18.000000000 +0200
+++ create.py.af 2010-09-13 16:59:48.000000000 +0200
@@ -1120,8 +1120,10 @@

# For case we don't have maxvcpus set but we have vcpus we preserve
# old behaviour
+ # te@artfiles.de: need to read vcpu_avail here since it isn't done later.
if not maxvcpus and vcpus:
config.append(['vcpus', vcpus])
+ config.append(['vcpu_avail', getattr(vals, 'vcpu_avail'])

def add_conf(n):
if hasattr(vals, n):

I would have filed a bug report for this but I have not found Xen4 on
Xen Bugzilla. Any hints on this?

Regards

Tim Evers
Artfiles GmbH

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Tim Evers writes ("[Xen-devel] Bug in xm config parsing code"):
> I think I've found a bug in xm/create.py:
> http://lists.xensource.com/archives/html/xen-changelog/2009-11/msg00132.html
> introduces the "maxvcpus" parameter. It states that

Your explanation is plausible. I have CC'd the author of the patch
you're talking about (changeset dc121a3c3a92).

Michal: would you like to have an opinion ? I'm inclined to apply
Tim's patch.

> + # te@artfiles.de: need to read vcpu_avail here since it isn't done later.

Tim: FYI, there is no for that kind of comment, as our revision
control system will keep track of these kind of things. The prose
part of your email will make a good checkin comment and allow future
readers to see exactly what was done why.

Also, you should normally provide a Signed-off-by. You do it like
this:
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
except with your name and email address. This indicates that you are
certifying the code as suitable (copyright-wise and so on) for
inclusion in Xen. You can find the precise meaning in the Linux
upstream kernel tree (Documentation/SubmittingPatches, copy below).

However for this trivial one-line change I'd be prepared to apply your
change without it, so there is no need for you to resend or sign off
now.

Thanks,
Ian.

>From Documentation/SubmittingPatches:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

--

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel