7:24 p.m.
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1201029734 28800
# Node ID 84f80a8040eb29109d0fce50b582582527c2e50e
# Parent 47438edf32be70e65bdb814ab609a70304fad2f4
Add dynamic VCPU adjustments
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 47438edf32be -r 84f80a8040eb libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Tue Jan 22 10:55:59 2008 -0800
+++ b/libxkutil/device_parsing.c Tue Jan 22 11:22:14 2008 -0800
@@ -897,11 +897,38 @@ static int change_memory(virDomainPtr do
return 1;
}
+static int change_vcpus(virDomainPtr dom, int delta)
+{
+ int count = 32;
+ int max;
+ virVcpuInfoPtr foo;
+
+ /* This is retarded, but I don't think there's an API to
+ * expose the total VCPU number otherwise
+ */
+ for (max = count; max == count; max += 32) {
+ foo = calloc(max, sizeof(*foo));
+ count = virDomainGetVcpus(dom, foo, max, NULL, 0);
+ free(foo);
+ }
+
+ CU_DEBUG("Changing VCPUs of %s from %i to %i",
+ virDomainGetName(dom),
+ count,
+ count + delta);
+
+ virDomainSetVcpus(dom, count + delta);
+
+ return 1;
+}
+
int attach_device(virDomainPtr dom, struct virt_device *dev)
{
if ((dev->type == VIRT_DEV_NET) ||
(dev->type == VIRT_DEV_DISK))
return _change_device(dom, dev, true);
+ else if (dev->type == VIRT_DEV_VCPU)
+ return change_vcpus(dom, 1);
CU_DEBUG("Unhandled device type %i", dev->type);
@@ -913,6 +940,8 @@ int detach_device(virDomainPtr dom, stru
if ((dev->type == VIRT_DEV_NET) ||
(dev->type == VIRT_DEV_DISK))
return _change_device(dom, dev, false);
+ else if (dev->type == VIRT_DEV_VCPU)
+ return change_vcpus(dom, -1);
CU_DEBUG("Unhandled device type %i", dev->type);
12:45 p.m.
Dan Smith wrote:
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1201029734 28800
# Node ID 84f80a8040eb29109d0fce50b582582527c2e50e
# Parent 47438edf32be70e65bdb814ab609a70304fad2f4
Add dynamic VCPU adjustments
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 47438edf32be -r 84f80a8040eb libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Tue Jan 22 10:55:59 2008 -0800
+++ b/libxkutil/device_parsing.c Tue Jan 22 11:22:14 2008 -0800
@@ -897,11 +897,38 @@ static int change_memory(virDomainPtr do
return 1;
}
+static int change_vcpus(virDomainPtr dom, int delta)
+{
+ int count = 32;
+ int max;
+ virVcpuInfoPtr foo;
+
+ /* This is retarded, but I don't think there's an API to
+ * expose the total VCPU number otherwise
+ */
I saw that virDomainInfo contains a field nrVirtCpu. Maybe this is an
alternative.
+ for (max = count; max == count; max += 32) {
+ foo = calloc(max, sizeof(*foo));
+ count = virDomainGetVcpus(dom, foo, max, NULL, 0);
Wouldn't the case "virDomainGetVcpus = 32" cause an endless loop ?
On
the other hand ... I can not quite follow the reason of this for loop,
which exits right after the first run in case of "count < 32" ?
+ free(foo);
+ }
+
+ CU_DEBUG("Changing VCPUs of %s from %i to %i",
+ virDomainGetName(dom),
+ count,
+ count + delta);
+
+ virDomainSetVcpus(dom, count + delta);
This function returns -1 in case of failure. Shouldn't this error case
be reported to the caller ? With the current setup the caller expects
that everything worked.
+
+ return 1;
+}
+
int attach_device(virDomainPtr dom, struct virt_device *dev)
{
if ((dev->type == VIRT_DEV_NET) ||
(dev->type == VIRT_DEV_DISK))
return _change_device(dom, dev, true);
+ else if (dev->type == VIRT_DEV_VCPU)
+ return change_vcpus(dom, 1);
CU_DEBUG("Unhandled device type %i", dev->type);
@@ -913,6 +940,8 @@ int detach_device(virDomainPtr dom, stru
if ((dev->type == VIRT_DEV_NET) ||
(dev->type == VIRT_DEV_DISK))
return _change_device(dom, dev, false);
+ else if (dev->type == VIRT_DEV_VCPU)
+ return change_vcpus(dom, -1);
CU_DEBUG("Unhandled device type %i", dev->type);
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
3:36 p.m.
HE> I saw that virDomainInfo contains a field nrVirtCpu. Maybe this is
HE> an alternative.
Okay, that was stupid of me :)
HE> Wouldn't the case "virDomainGetVcpus = 32" cause an endless loop ?
HE> On the other hand ... I can not quite follow the reason of this
HE> for loop, which exits right after the first run in case of "count
HE> < 32" ?
No, max gets incremented by 32 each time. The only time we loop again
is if count is equal to max. So, if there are, say 65 vcpus, then the
first time through, we set max=32, and we get back our maximum. That
means count == max == 32. The next time, max is 64, and we get back
64 as well. The next time, max is 96, but we only get back 65, so we
exit the loop.
However, as you pointed out, there is a *much* better way. I just
didn't see it grepping through the API, but I was looking for a
function instead of virDomainInfo, so I missed it :)
HE> This function returns -1 in case of failure. Shouldn't this error
HE> case be reported to the caller ? With the current setup the caller
HE> expects that everything worked.
The caller actually shouldn't care if this fails, because this is only
the dynamic part. If this fails, we still re-write the XML as
expected. However, for the sake of correctness, you're absolutely
right.
Thanks a lot!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:49 p.m.
Dan Smith wrote:
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1201029734 28800
# Node ID 84f80a8040eb29109d0fce50b582582527c2e50e
# Parent 47438edf32be70e65bdb814ab609a70304fad2f4
Add dynamic VCPU adjustments
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 47438edf32be -r 84f80a8040eb libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Tue Jan 22 10:55:59 2008 -0800
+++ b/libxkutil/device_parsing.c Tue Jan 22 11:22:14 2008 -0800
@@ -897,11 +897,38 @@ static int change_memory(virDomainPtr do
return 1;
}
+static int change_vcpus(virDomainPtr dom, int delta)
+{
+ int count = 32;
+ int max;
+ virVcpuInfoPtr foo;
+
+ /* This is retarded, but I don't think there's an API to
+ * expose the total VCPU number otherwise
+ */
+ for (max = count; max == count; max += 32) {
+ foo = calloc(max, sizeof(*foo));
+ count = virDomainGetVcpus(dom, foo, max, NULL, 0);
+ free(foo);
+ }
I just spent the last ten minutes trying to figure out how to say that I
have no idea what's going on with that loop without it sounding like I
was calling at least one of use stupid. So far, the best I've come up
with is, "I have no idea what's going on with that loop." Judging by
your comment, you aren't exactly thrilled with it, and your track record
indicates you certainly wouldn't do something like this without a good
reason, but if you could explain things a little more I'd appreciate it.
The rest looks good to me.
--
-Jay
6085
days inactive
6086
days old
3 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon