8:43 p.m.
# HG changeset patch
# User Kaitlin Rupert <karupert(a)us.ibm.com>
# Date 1199734960 28800
# Node ID abcb596636c9530009756b1aa1e8900a2f01d10e
# Parent ea65740aa1ffe73b47a4a14c8a853dbb3f040016
Fix get_graphics_device() - returns error if graphics tag is missing.
The graphics tag isn't mandatory, so we shouldn't return an error here.
The same problem exists with get_emu_device().
Signed-off-by: Kaitlin Rupert <karupert(a)us.ibm.com>
diff -r ea65740aa1ff -r abcb596636c9 libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Fri Jan 04 12:53:44 2008 -0800
+++ b/libxkutil/device_parsing.c Mon Jan 07 11:42:40 2008 -0800
@@ -395,7 +395,7 @@ static int get_emu_device(virDomainPtr d
free(xml);
- return ret;
+ return 1;
}
static int get_graphics_device(virDomainPtr dom, struct virt_device **dev)
@@ -416,7 +416,7 @@ static int get_graphics_device(virDomain
free(xml);
- return ret;
+ return 1;
}
int get_disk_devices(virDomainPtr dom, struct virt_device **list)
1:26 p.m.
Kaitlin Rupert wrote:
# HG changeset patch
# User Kaitlin Rupert <karupert(a)us.ibm.com>
# Date 1199734960 28800
# Node ID abcb596636c9530009756b1aa1e8900a2f01d10e
# Parent ea65740aa1ffe73b47a4a14c8a853dbb3f040016
Fix get_graphics_device() - returns error if graphics tag is missing.
The graphics tag isn't mandatory, so we shouldn't return an error here.
The same problem exists with get_emu_device().
Signed-off-by: Kaitlin Rupert <karupert(a)us.ibm.com>
diff -r ea65740aa1ff -r abcb596636c9 libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Fri Jan 04 12:53:44 2008 -0800
+++ b/libxkutil/device_parsing.c Mon Jan 07 11:42:40 2008 -0800
@@ -395,7 +395,7 @@ static int get_emu_device(virDomainPtr d
free(xml);
- return ret;
+ return 1;
}
static int get_graphics_device(virDomainPtr dom, struct virt_device **dev)
@@ -416,7 +416,7 @@ static int get_graphics_device(virDomain
free(xml);
- return ret;
+ return 1;
}
int get_disk_devices(virDomainPtr dom, struct virt_device **list)
If the return code of these two functions does not matter, couldn't we
also make a void function out of them ? This could avoid the impression
that the result of these functions matters for the return code of the
calling function, like in get_dominfo().
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
3:56 p.m.
KR> - return ret;
KR> + return 1;
This will cause the do_parse() function to keep an empty (and
uninitialized) device struct on the list, because of this:
if (do_real_parse(dev_nodes[devidx], &list[lstidx]))
lstidx++;
Which is bad. I think we need to keep the conditional return value
From those individual functions. I think the problem lies just with
this bit in get_dominfo():
ret = get_emu_device(dom, &(*dominfo)->dev_emu);
ret = get_graphics_device(dom, &(*dominfo)->dev_graphics);
Which was me thinking that I'd capture the return value and maybe
check it later, but ignore for now. The problem is that we return ret
a few lines down, which makes the function fail if one of the two
above calls fail.
So, I think the solution here for now is to either do something
meaningful with ret (not sure what that would be), or just remove
'ret =' from each call and truly ignore the status of those.
Also,
KR> - return ret;
KR> + return 1;
These functions now return a bool instead of an int. Maybe you need
to update your tree?
Thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
4:15 p.m.
Dan Smith wrote:
KR> - return ret;
KR> + return 1;
This will cause the do_parse() function to keep an empty (and
uninitialized) device struct on the list, because of this:
> if (do_real_parse(dev_nodes[devidx], &list[lstidx]))
> lstidx++;
Ah good point. Yep, I didn't notice this.
Which is bad. I think we need to keep the conditional return value
From those individual functions. I think the problem lies just with
this bit in get_dominfo():
> ret = get_emu_device(dom, &(*dominfo)->dev_emu);
> ret = get_graphics_device(dom, &(*dominfo)->dev_graphics);
Which was me thinking that I'd capture the return value and maybe
check it later, but ignore for now. The problem is that we return ret
a few lines down, which makes the function fail if one of the two
above calls fail.
So, I think the solution here for now is to either do something
meaningful with ret (not sure what that would be), or just remove
'ret =' from each call and truly ignore the status of those.
I'd vote for not capturing the return from the get_emu_device() and
get_graphics_device() in get_dominfo().
Trying to handle the return in a meaningful way would probably not be
obvious and would need a comment..
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
6098
days inactive
6099
days old
3 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert