# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1201029734 28800 # Node ID 84f80a8040eb29109d0fce50b582582527c2e50e # Parent 47438edf32be70e65bdb814ab609a70304fad2f4 Add dynamic VCPU adjustments Signed-off-by: Dan Smith <danms@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);
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1201029734 28800 # Node ID 84f80a8040eb29109d0fce50b582582527c2e50e # Parent 47438edf32be70e65bdb814ab609a70304fad2f4 Add dynamic VCPU adjustments
Signed-off-by: Dan Smith <danms@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
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@us.ibm.com
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1201029734 28800 # Node ID 84f80a8040eb29109d0fce50b582582527c2e50e # Parent 47438edf32be70e65bdb814ab609a70304fad2f4 Add dynamic VCPU adjustments
Signed-off-by: Dan Smith <danms@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
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon