* [PATCH] Fix assignment of devid value returned from libxl__device_nextid
@ 2013-06-25 22:02 Jim Fehlig
2013-06-26 9:41 ` George Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Jim Fehlig @ 2013-06-25 22:02 UTC (permalink / raw)
To: xen-devel
Commit 5420f265 has some misplaced parenthesis that caused devid
to be assigned 1 or 0 based on checking return value of
libxl__device_nextid < 0, e.g.
devid = libxl__device_nextid(...) < 0
This works when only one instance of a given device type exists, but
subsequent devices of the same type will also have a devid = 1 if
libxl__device_nextid succeeds. Fix by checking the value assigned to
devid, e.g.
(devid = libxl__device_nextid(...)) < 0
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
tools/libxl/libxl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0612d85..1bbb990 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1781,7 +1781,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
back = flexarray_make(gc, 16, 1);
if (vtpm->devid == -1) {
- if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm") < 0)) {
+ if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm")) < 0) {
rc = ERROR_FAIL;
goto out;
}
@@ -2877,7 +2877,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
back = flexarray_make(gc, 18, 1);
if (nic->devid == -1) {
- if ((nic->devid = libxl__device_nextid(gc, domid, "vif") < 0)) {
+ if ((nic->devid = libxl__device_nextid(gc, domid, "vif")) < 0) {
rc = ERROR_FAIL;
goto out;
}
@@ -3258,7 +3258,7 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
back = flexarray_make(gc, 16, 1);
if (vkb->devid == -1) {
- if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb") < 0)) {
+ if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb")) < 0) {
rc = ERROR_FAIL;
goto out;
}
@@ -3359,7 +3359,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
back = flexarray_make(gc, 16, 1);
if (vfb->devid == -1) {
- if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb") < 0)) {
+ if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb")) < 0) {
rc = ERROR_FAIL;
goto out;
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Fix assignment of devid value returned from libxl__device_nextid
2013-06-25 22:02 [PATCH] Fix assignment of devid value returned from libxl__device_nextid Jim Fehlig
@ 2013-06-26 9:41 ` George Dunlap
2013-06-26 16:24 ` Ian Campbell
2013-06-26 16:42 ` Jim Fehlig
0 siblings, 2 replies; 6+ messages in thread
From: George Dunlap @ 2013-06-26 9:41 UTC (permalink / raw)
To: Jim Fehlig; +Cc: xen-devel@lists.xen.org
On Tue, Jun 25, 2013 at 11:02 PM, Jim Fehlig <jfehlig@suse.com> wrote:
> Commit 5420f265 has some misplaced parenthesis that caused devid
> to be assigned 1 or 0 based on checking return value of
> libxl__device_nextid < 0, e.g.
>
> devid = libxl__device_nextid(...) < 0
>
> This works when only one instance of a given device type exists, but
> subsequent devices of the same type will also have a devid = 1 if
> libxl__device_nextid succeeds. Fix by checking the value assigned to
> devid, e.g.
>
> (devid = libxl__device_nextid(...)) < 0
>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Both for 4.3, and as a patch in general:
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> tools/libxl/libxl.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 0612d85..1bbb990 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1781,7 +1781,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
> back = flexarray_make(gc, 16, 1);
>
> if (vtpm->devid == -1) {
> - if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm") < 0)) {
> + if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm")) < 0) {
> rc = ERROR_FAIL;
> goto out;
> }
> @@ -2877,7 +2877,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
> back = flexarray_make(gc, 18, 1);
>
> if (nic->devid == -1) {
> - if ((nic->devid = libxl__device_nextid(gc, domid, "vif") < 0)) {
> + if ((nic->devid = libxl__device_nextid(gc, domid, "vif")) < 0) {
> rc = ERROR_FAIL;
> goto out;
> }
> @@ -3258,7 +3258,7 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
> back = flexarray_make(gc, 16, 1);
>
> if (vkb->devid == -1) {
> - if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb") < 0)) {
> + if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb")) < 0) {
> rc = ERROR_FAIL;
> goto out;
> }
> @@ -3359,7 +3359,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
> back = flexarray_make(gc, 16, 1);
>
> if (vfb->devid == -1) {
> - if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb") < 0)) {
> + if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb")) < 0) {
> rc = ERROR_FAIL;
> goto out;
> }
> --
> 1.8.0.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix assignment of devid value returned from libxl__device_nextid
2013-06-26 9:41 ` George Dunlap
@ 2013-06-26 16:24 ` Ian Campbell
2013-06-26 16:42 ` Jim Fehlig
1 sibling, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2013-06-26 16:24 UTC (permalink / raw)
To: George Dunlap; +Cc: Jim Fehlig, xen-devel@lists.xen.org
On Wed, 2013-06-26 at 10:41 +0100, George Dunlap wrote:
> On Tue, Jun 25, 2013 at 11:02 PM, Jim Fehlig <jfehlig@suse.com> wrote:
> > Commit 5420f265 has some misplaced parenthesis that caused devid
> > to be assigned 1 or 0 based on checking return value of
> > libxl__device_nextid < 0, e.g.
> >
> > devid = libxl__device_nextid(...) < 0
> >
> > This works when only one instance of a given device type exists, but
> > subsequent devices of the same type will also have a devid = 1 if
> > libxl__device_nextid succeeds. Fix by checking the value assigned to
> > devid, e.g.
> >
> > (devid = libxl__device_nextid(...)) < 0
This is why I hate combined assignment & logic!
> > Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>
> Both for 4.3, and as a patch in general:
>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked + applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assignment of devid value returned from libxl__device_nextid
2013-06-26 9:41 ` George Dunlap
2013-06-26 16:24 ` Ian Campbell
@ 2013-06-26 16:42 ` Jim Fehlig
2013-06-26 16:49 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Jim Fehlig @ 2013-06-26 16:42 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel@lists.xen.org
George Dunlap wrote:
> On Tue, Jun 25, 2013 at 11:02 PM, Jim Fehlig <jfehlig@suse.com> wrote:
>
>> Commit 5420f265 has some misplaced parenthesis that caused devid
>> to be assigned 1 or 0 based on checking return value of
>> libxl__device_nextid < 0, e.g.
>>
>> devid = libxl__device_nextid(...) < 0
>>
>> This works when only one instance of a given device type exists, but
>> subsequent devices of the same type will also have a devid = 1 if
>> libxl__device_nextid succeeds. Fix by checking the value assigned to
>> devid, e.g.
>>
>> (devid = libxl__device_nextid(...)) < 0
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>
>
> Both for 4.3, and as a patch in general:
>
Noticed I have the commit msg a tad bit wrong. It should read
"...devices of the same type will also have a devid = 0 if..." since
libxl__device_nextid(...) < 0 is false when libxl__device_nextid()
succeeds. Also, perhaps I should add that xl is not affected since it
supplies devid, but apps such as libvirt that allow libxl to fill in
devid are certainly affected when there is more than 1 of the same
device type. Should I submit a V2 with an improved commit msg?
Regards,
Jim
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
>
>> ---
>> tools/libxl/libxl.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 0612d85..1bbb990 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1781,7 +1781,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
>> back = flexarray_make(gc, 16, 1);
>>
>> if (vtpm->devid == -1) {
>> - if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm") < 0)) {
>> + if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm")) < 0) {
>> rc = ERROR_FAIL;
>> goto out;
>> }
>> @@ -2877,7 +2877,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
>> back = flexarray_make(gc, 18, 1);
>>
>> if (nic->devid == -1) {
>> - if ((nic->devid = libxl__device_nextid(gc, domid, "vif") < 0)) {
>> + if ((nic->devid = libxl__device_nextid(gc, domid, "vif")) < 0) {
>> rc = ERROR_FAIL;
>> goto out;
>> }
>> @@ -3258,7 +3258,7 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
>> back = flexarray_make(gc, 16, 1);
>>
>> if (vkb->devid == -1) {
>> - if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb") < 0)) {
>> + if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb")) < 0) {
>> rc = ERROR_FAIL;
>> goto out;
>> }
>> @@ -3359,7 +3359,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
>> back = flexarray_make(gc, 16, 1);
>>
>> if (vfb->devid == -1) {
>> - if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb") < 0)) {
>> + if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb")) < 0) {
>> rc = ERROR_FAIL;
>> goto out;
>> }
>> --
>> 1.8.0.1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix assignment of devid value returned from libxl__device_nextid
2013-06-26 16:42 ` Jim Fehlig
@ 2013-06-26 16:49 ` Ian Campbell
2013-06-26 17:05 ` Jim Fehlig
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-06-26 16:49 UTC (permalink / raw)
To: Jim Fehlig; +Cc: George Dunlap, xen-devel@lists.xen.org
On Wed, 2013-06-26 at 10:42 -0600, Jim Fehlig wrote:
> George Dunlap wrote:
> > On Tue, Jun 25, 2013 at 11:02 PM, Jim Fehlig <jfehlig@suse.com> wrote:
> >
> >> Commit 5420f265 has some misplaced parenthesis that caused devid
> >> to be assigned 1 or 0 based on checking return value of
> >> libxl__device_nextid < 0, e.g.
> >>
> >> devid = libxl__device_nextid(...) < 0
> >>
> >> This works when only one instance of a given device type exists, but
> >> subsequent devices of the same type will also have a devid = 1 if
> >> libxl__device_nextid succeeds. Fix by checking the value assigned to
> >> devid, e.g.
> >>
> >> (devid = libxl__device_nextid(...)) < 0
> >>
> >> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> >>
> >
> > Both for 4.3, and as a patch in general:
> >
>
> Noticed I have the commit msg a tad bit wrong. It should read
> "...devices of the same type will also have a devid = 0 if..." since
> libxl__device_nextid(...) < 0 is false when libxl__device_nextid()
> succeeds. Also, perhaps I should add that xl is not affected since it
> supplies devid, but apps such as libvirt that allow libxl to fill in
> devid are certainly affected when there is more than 1 of the same
> device type. Should I submit a V2 with an improved commit msg?
I'm afraid I have already committed.
Not to worry.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix assignment of devid value returned from libxl__device_nextid
2013-06-26 16:49 ` Ian Campbell
@ 2013-06-26 17:05 ` Jim Fehlig
0 siblings, 0 replies; 6+ messages in thread
From: Jim Fehlig @ 2013-06-26 17:05 UTC (permalink / raw)
To: Ian Campbell; +Cc: George Dunlap, xen-devel@lists.xen.org
Ian Campbell wrote:
> On Wed, 2013-06-26 at 10:42 -0600, Jim Fehlig wrote:
>
>> George Dunlap wrote:
>>
>>> On Tue, Jun 25, 2013 at 11:02 PM, Jim Fehlig <jfehlig@suse.com> wrote:
>>>
>>>
>>>> Commit 5420f265 has some misplaced parenthesis that caused devid
>>>> to be assigned 1 or 0 based on checking return value of
>>>> libxl__device_nextid < 0, e.g.
>>>>
>>>> devid = libxl__device_nextid(...) < 0
>>>>
>>>> This works when only one instance of a given device type exists, but
>>>> subsequent devices of the same type will also have a devid = 1 if
>>>> libxl__device_nextid succeeds. Fix by checking the value assigned to
>>>> devid, e.g.
>>>>
>>>> (devid = libxl__device_nextid(...)) < 0
>>>>
>>>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>>>
>>>>
>>> Both for 4.3, and as a patch in general:
>>>
>>>
>> Noticed I have the commit msg a tad bit wrong. It should read
>> "...devices of the same type will also have a devid = 0 if..." since
>> libxl__device_nextid(...) < 0 is false when libxl__device_nextid()
>> succeeds. Also, perhaps I should add that xl is not affected since it
>> supplies devid, but apps such as libvirt that allow libxl to fill in
>> devid are certainly affected when there is more than 1 of the same
>> device type. Should I submit a V2 with an improved commit msg?
>>
>
> I'm afraid I have already committed.
>
Yep, saw that just after I sent this message :).
> Not to worry.
>
Right. The existing msg at least adequately describes the problem.
Regards,
Jim
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-26 17:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-25 22:02 [PATCH] Fix assignment of devid value returned from libxl__device_nextid Jim Fehlig
2013-06-26 9:41 ` George Dunlap
2013-06-26 16:24 ` Ian Campbell
2013-06-26 16:42 ` Jim Fehlig
2013-06-26 16:49 ` Ian Campbell
2013-06-26 17:05 ` Jim Fehlig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).