[libvirt] The config of IP address for 'macvtap' network
Hi guys, I know the macvtap network is supported by libvirt as forward mode 'passthrough', I wonder is there anyway to configure the IP address for its interface? For example, If I create a network as below: network namevdsm-testnet/name uuid31f6b3b3-e959-0dd1-ad3a-bf95db660415/uuid forward dev='eth0.8' mode='passthrough' interface dev='eth0.8'/ /forward /network For now, I have to set the IP address by 'ifconfig eth0.8 XXX.XXX.XXX.XXX' after defining the network. How can I set IP address for this VLAN device 'eth0.8' by libvirt, I mean does libvirt support to assign IP address in this mode now? If does, what is the xml format for it? It'd appreciate a lot if anybody could read my post and give me some suggestions! -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] The config of IP address for 'macvtap' network
On 05/15/2012 05:33 PM, Michal Privoznik wrote: On 15.05.2012 10:57, Lei Li wrote: Hi guys, I know the macvtap network is supported by libvirt as forward mode 'passthrough', I wonder is there anyway to configure the IP address for its interface? For example, If I create a network as below: network namevdsm-testnet/name uuid31f6b3b3-e959-0dd1-ad3a-bf95db660415/uuid forward dev='eth0.8' mode='passthrough' interface dev='eth0.8'/ /forward /network For now, I have to set the IP address by 'ifconfig eth0.8 XXX.XXX.XXX.XXX' after defining the network. How can I set IP address for this VLAN device 'eth0.8' by libvirt, I mean does libvirt support to assign IP address in this mode now? If does, what is the xml format for it? It'd appreciate a lot if anybody could read my post and give me some suggestions! I suppose adding: ip address='192.168.123.1' netmask='255.255.255.0'/ intonetwork/ should work, doesn't it? Thank you very much for your reply! I did try to add IP address like this, but seems it didn't work... network namevdsm-testnet/name uuid9b1b162c-3056-de63-6785-30b45abf24cf/uuid forward dev='eth0.6' mode='passthrough' interface dev='eth0.6'/ /forward ip address='192.168.123.1' netmask='255.255.255.0' /ip /network or ... ... ip address='192.168.123.1' netmask='255.255.255.0'/ ... Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] The config of IP address for 'macvtap' network
On 05/15/2012 11:12 PM, Laine Stump wrote: On 05/15/2012 05:33 AM, Michal Privoznik wrote: On 15.05.2012 10:57, Lei Li wrote: Hi guys, I know the macvtap network is supported by libvirt as forward mode 'passthrough', I wonder is there anyway to configure the IP address for its interface? For example, If I create a network as below: network namevdsm-testnet/name uuid31f6b3b3-e959-0dd1-ad3a-bf95db660415/uuid forward dev='eth0.8' mode='passthrough' interface dev='eth0.8'/ /forward /network For now, I have to set the IP address by 'ifconfig eth0.8 XXX.XXX.XXX.XXX' after defining the network. If only the guest will use the interface, that isn't necessary. As a matter of fact, once an interface has been assigned to a guest using macvtap passthrough mode, the host *can't* use the interface (it can continue to use it in the other macvtap modes, however), so it's completely pointless to configure an IP address for that interface on the host. How can I set IP address for this VLAN device 'eth0.8' by libvirt, I mean does libvirt support to assign IP address in this mode now? If does, what is the xml format for it? A network device being used for a guest macvtap connection only needs an IP address set on the host if the host will also be using that interface - the guest does not magically acquire/use the IP address that has been set on the host, it needs its own IP address, configured on the guest in the same fashion you would configure any other guest interface (keep in mind that even in macvtap 'bridge' mode, the host and guest cannot communicate with each other via a macvtap interface). Otherwise, it's enough for the interface to be defined on the host (it may also be necessary for it to be up if it's a vlan device - I haven't tried macvtap with vlans). If you are using RHEL or Fedora, you can do any/all of that configuration with the virsh iface-define command. For example, here is the xml file that would define an eth0.8 interface on a host (This is *not* theinterface element of a domain configuration): interface type=vlan name=eth0.8 start mode=none/ protocol family=ipv4 ip address=192.168.43.1/ /protocol vlan tag=8 interface name=eth0/ /vlan /interface To get this defined in the system, you would use: virsh iface-define eth0.8.xml But again, it seems doubtful this is what you really want. It'd appreciate a lot if anybody could read my post and give me some suggestions! I suppose adding: ip address='192.168.123.1' netmask='255.255.255.0'/ intonetwork/ should work, doesn't it? No. The IP address used by the host for that interface (if any) should be configured in the normal manner of the host system. The IP address used by any guests using this interface / network based on this interface should be configured in the guest just as you would any other interface (if the guest is configured to get its IP address from DHCP, then you would need to have a DHCP server running *on a different host* connected to the .8 vlan. (BTW, note that in passthrough mode, only a single guest can connect to each physical device, so having anetwork defined for passthrough mode is mostly only useful if you have a pool of devices available. The way you have it defined above, only a single guest would be able to use that network.) Hi Laine, / /Thank you for your detailed reply! The vdsm support bridgless network by libvirt through passthrough mode, it just define a network by the xml format: network namevdsm-XXX/name uuid.../uuid forward dev=... mode='passthrough interface dev=.../ /forward /network I want to let IP address config for the host network enabled. It doesn't have to be connected by a VLAN device,a simpler example, If I create a bridgeless network by passthrough mode, its interface is eth0. What is the xml format for the host to let the IP address configuration enabled? Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Provide a helper method virDomainLiveConfigHelperMethod
Changes since v1 - With Eric's comments squashed in. This chunk of code below repeated in several functions, factor it into a helper method virDomainLiveConfigHelperMethod to eliminate duplicated code based on Eric and Adam's suggestion. I have tested it for all the relevant APIs changed. Signed-off-by: Eric Blake ebl...@redhat.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 51 src/conf/domain_conf.h |7 + src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 288 -- 4 files changed, 81 insertions(+), 266 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d68ab10..da98a22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1670,6 +1670,57 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* + * Helper method for --current --live --config option, and check with + * whether domain is active or can get persistent domain configuration. + * + * Return 0 if success, also change the flags and get the persistent + * domain configuration if needed. Return -1 on error. + */ +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, +virDomainObjPtr dom, +unsigned int *flags, +virDomainDefPtr *persistentDef) +{ +bool isActive; +int ret = -1; + +isActive = virDomainObjIsActive(dom); + +if ((*flags (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == +VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +*flags |= VIR_DOMAIN_AFFECT_LIVE; +else +*flags |= VIR_DOMAIN_AFFECT_CONFIG; +} + +if (!isActive (*flags VIR_DOMAIN_AFFECT_LIVE)) { +virDomainReportError(VIR_ERR_OPERATION_INVALID, %s, + _(domain is not running)); +goto cleanup; +} + +if (*flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!dom-persistent) { +virDomainReportError(VIR_ERR_OPERATION_INVALID, %s, + _(cannot change persistent config of a transient domain)); +goto cleanup; +} +if (!(*persistentDef = virDomainObjGetPersistentDef(caps, dom))) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Get persistent config failed)); +goto cleanup; +} +} + +ret = 0; + +cleanup: +return ret; +} + +/* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else * is either waiting for 'dom' or still using it diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d6ed898..3229a6f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1737,6 +1737,13 @@ int virDomainObjSetDefTransient(virCapsPtr caps, virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainObjPtr domain); + +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, +virDomainObjPtr dom, +unsigned int *flags, +virDomainDefPtr *persistentDef); + virDomainDefPtr virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a81c230..48ffdf2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -358,6 +358,7 @@ virDomainLifecycleCrashTypeFromString; virDomainLifecycleCrashTypeToString; virDomainLifecycleTypeFromString; virDomainLifecycleTypeToString; +virDomainLiveConfigHelperMethod; virDomainLoadAllConfigs; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10a289e..aa92573 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1822,12 +1822,6 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, isActive = virDomainObjIsActive(vm); -if (flags == VIR_DOMAIN_AFFECT_CURRENT) { -if (isActive) -flags = VIR_DOMAIN_AFFECT_LIVE; -else -flags = VIR_DOMAIN_AFFECT_CONFIG; -} if (flags == VIR_DOMAIN_MEM_MAXIMUM) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_MEM_MAXIMUM; @@ -1835,21 +1829,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, flags = VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_MEM_MAXIMUM; } -if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { -qemuReportError(VIR_ERR_OPERATION_INVALID, -%s, _(domain is not running)); +if (virDomainLiveConfigHelperMethod(driver-caps, vm, flags, persistentDef) 0) goto endjob; -} - -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -if (!vm-persistent) { -qemuReportError(VIR_ERR_OPERATION_INVALID, %s
[libvirt] [PATCH] Provide a helper method virDomainLiveHelperMethod
This chunk of code below repeated in several functions, factor it into a helper method virDomainLiveHelperMethod to eliminate duplicated code based on Eric and Adam's suggestion. I have tested it for all the relevant APIs changed. isActive = virDomainObjIsActive(vm); if (flags == VIR_DOMAIN_AFFECT_CURRENT) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE; else flags = VIR_DOMAIN_AFFECT_CONFIG; } if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, %s, _(domain is not running)); goto endjob; } if (flags VIR_DOMAIN_AFFECT_CONFIG) { if (!vm-persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, %s, _(cannot change persistent config of a transient domain)); goto endjob; } if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) goto endjob; } Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 47 src/conf/domain_conf.h |7 + src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 288 -- 4 files changed, 77 insertions(+), 266 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 75e51a0..12ea12d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1670,6 +1670,53 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* + * Helper method for --current --live --config option, and check with + * whether domain is active or can get persistent domain configuration. + * + * Return 0 if success, also change the flags and get the persistent + * domain configuration if needed. Return -1 on error. + */ +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, +virDomainObjPtr dom, +unsigned int *flags, +virDomainDefPtr *persistentDef) +{ +bool isActive; +int ret = 0; + +isActive = virDomainObjIsActive(dom); + +if (*flags == VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +*flags = VIR_DOMAIN_AFFECT_LIVE; +else +*flags = VIR_DOMAIN_AFFECT_CONFIG; +} + +if (!isActive (*flags VIR_DOMAIN_AFFECT_LIVE)) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(domain is not running)); +ret = -1; +} + +if (*flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!dom-persistent) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot change persistent config of a transient domain)); +ret = -1; +} +if (!(*persistentDef = virDomainObjGetPersistentDef(caps, dom))) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Get persistent config failed)); +ret = -1; +} +} + +return ret; +} + +/* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else * is either waiting for 'dom' or still using it diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d6ed898..3229a6f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1737,6 +1737,13 @@ int virDomainObjSetDefTransient(virCapsPtr caps, virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainObjPtr domain); + +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, +virDomainObjPtr dom, +unsigned int *flags, +virDomainDefPtr *persistentDef); + virDomainDefPtr virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99a1099..5962d93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -358,6 +358,7 @@ virDomainLifecycleCrashTypeFromString; virDomainLifecycleCrashTypeToString; virDomainLifecycleTypeFromString; virDomainLifecycleTypeToString; +virDomainLiveConfigHelperMethod; virDomainLoadAllConfigs; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e5ed9a..9991383 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1822,12 +1822,6 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, isActive = virDomainObjIsActive(vm); -if (flags == VIR_DOMAIN_AFFECT_CURRENT) { -if (isActive) -flags = VIR_DOMAIN_AFFECT_LIVE; -else -flags = VIR_DOMAIN_AFFECT_CONFIG; -} if (flags == VIR_DOMAIN_MEM_MAXIMUM) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_MEM_MAXIMUM; @@ -1835,21 +1829,8 @@ static int
Re: [libvirt] [PATCHv6 4/7] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
On 12/01/2011 02:15 AM, Eric Blake wrote: On 11/23/2011 02:44 PM, Eric Blake wrote: From: Lei Lili...@linux.vnet.ibm.com Implement the block I/O throttle setting and getting support to qemu driver. Signed-off-by: Lei Lili...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Eric Blakeebl...@redhat.com +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!vm-persistent) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(cannot change persistent config of a transient domain)); +goto endjob; +} +if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) +goto endjob; +} + + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +int idx = virDomainDiskIndexByName(vm-def, disk, true); Oops - this should be on persistentDef, not vm-def. +if (i 0) +goto endjob; +persistentDef-disks[idx]-blkdeviotune = info; And this assignment should be delayed... +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device,info); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { to here, after we know the live change (if any) took place. Not to mention that we must not get here if the live change failed. Here's what I'm squashing in: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 698a961..ce4cba1 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -11080,6 +11080,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, int ret = -1; int i; bool isActive; +int idx = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -11126,6 +11127,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) goto endjob; +idx = virDomainDiskIndexByName(persistentDef, disk, true); +if (i 0) +goto endjob; } for (i = 0; i nparams; i++) { @@ -11177,22 +11181,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -sa_assert(persistentDef); -int idx = virDomainDiskIndexByName(vm-def, disk, true); -if (i 0) -goto endjob; -persistentDef-disks[idx]-blkdeviotune = info; -} - if (flags VIR_DOMAIN_AFFECT_LIVE) { priv = vm-privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv-mon, device,info); qemuDomainObjExitMonitorWithDriver(driver, vm); } +if (ret 0) +goto endjob; Hi Eric, Thanks again for your kind help! An error occurred when I test it. virsh #blkdeviotune kvm vda --total_bytes_sec 8 --config error: Unable to change block I/O throttle error: An error occurred, but the cause is unknown I dig into the code, here is a logic error. The initial value of ret = -1, if just set --config, it will goto endjob directly without doing its really job here. I guess the purpose you add these two lines is to check if live option succeeded, how about just get rid of these two lines. since the relevant check has already done in qemu monitor code. If live option failed it will return ret = -1 with error report, and if ret 0, src/libvirt.c will goto error. I submit a patch based on above to avoid this error. if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef idx= 0); +persistentDef-disks[idx]-blkdeviotune = info; ret = virDomainSaveConfig(driver-configDir, persistentDef); if (ret 0) { qemuReportError(VIR_ERR_OPERATION_INVALID, %s, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix a logic error for setting block I/O
Fix a logic error, the initial value of ret = -1, if just set --config, it will goto endjob directly without doing its really job here. Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed90c66..8ea6f48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11234,8 +11234,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info); qemuDomainObjExitMonitorWithDriver(driver, vm); } -if (ret 0) -goto endjob; if (flags VIR_DOMAIN_AFFECT_CONFIG) { sa_assert(persistentDef idx = 0); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
On 11/28/2011 11:24 PM, Adam Litke wrote: On Wed, Nov 23, 2011 at 10:41:32AM -0700, Eric Blake wrote: Hmm - passing nparams==1 to set just read_bytes_sec has the side effect of wiping out any existing total_iops_sec, even though those two parameters seem somewhat orthogonal, all because we zero-initialized the struct that we pass on to the monitor command. Is that intended? I can live with it (but it probably ought to be documented), but we may want to consider being more flexible, by using '0' to clear a previous limit, but initializing to '-1' to imply the limit does not change. Then the qemu_monitor_json code should only emit the arguments that are actually being changed, rather than blindly always outputting 6 parameters even when the user only passed in one parameter. But I'm okay delaying that to a separate patch based on whether others think it would be a good improvement. +1. I believe I had pointed this out previously as well (albeit not as concisely as this). Well, here is the description of block I/O throttling command 'block_io_set_throttle' in qmp-commands.hx. EQMP { .name = block_set_io_throttle, .args_type = device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?, .params = device [bps] [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr], .help = change I/O throttle limits for a block drive, .user_print = monitor_user_noop, .mhandler.cmd_new = do_block_set_io_throttle, }, SQMP block_set_io_throttle Change I/O throttle limits for a block drive. Arguments: - device: device name (json-string) - bps: total throughput limit in bytes per second(json-int, optional) - bps_rd: read throughput limit in bytes per second(json-int, optional) - bps_wr: read throughput limit in bytes per second(json-int, optional) - iops: total I/O operations per second(json-int, optional) - iops_rd: read I/O operations per second(json-int, optional) - iops_wr: write I/O operations per second(json-int, optional) Example: - { execute: block_set_io_throttle, arguments: { device: virtio0, bps: 100, bps_rd: 0, bps_wr: 0, iops: 0, iops_rd: 0, iops_wr: 0 } } - { return: {} } This qmp command need all these 6 parameters at one time in qemu, so zero-initialized the struct to meet If there is no setting value for some of the fields. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8 v5] Summary on block IO throttle
On 11/22/2011 08:14 AM, Eric Blake wrote: On 11/15/2011 02:02 AM, Lei Li wrote: Changes since V3 - Use virTypedParameterPtr instead of specific struct in libvirt pulic API. - Relevant changes to remote driver, qemu driver, python support and virsh. Hmm, you didn't summarize changes since v4. Sorry for not noticing the v5 a bit sooner (the thread is getting a bit long, so this got buried in my inbox). Hopefully we are closer to getting this applied; I'd certainly like to see it in 0.9.8. Hi Eric, Just a little minor changes as you mentioned in V4, so I didn't write it into summary change log. - Add device shorthand as a choice option to Fully-qualified name in the definition in src/libvirt.c. - Move the added name of public API to new section LIBVIRT_0.9.8 in src/libvirt_public.syms. Yeah, I hope so. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 101 +- src/conf/domain_conf.h | 12 ++ src/qemu/qemu_command.c | 33 +++ src/util/xml.h |2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..a157b80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child-next; } +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +if (virXPathULongLong(string(./devices/disk/iotune/total_bytes_sec), + ctxt, def-blkdeviotune.total_bytes_sec) 0) { +def-blkdeviotune.total_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_bytes_sec), + ctxt, def-blkdeviotune.read_bytes_sec) 0) { +def-blkdeviotune.read_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_bytes_sec), + ctxt, def-blkdeviotune.write_bytes_sec) 0) { +def-blkdeviotune.write_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + + if (virXPathULongLong(string(./devices/disk/iotune/total_iops_sec), + ctxt, def-blkdeviotune.total_iops_sec) 0) { +def-blkdeviotune.total_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_iops_sec), + ctxt, def-blkdeviotune.read_iops_sec) 0) { +def-blkdeviotune.read_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_iops_sec), + ctxt, def-blkdeviotune.write_iops_sec) 0) { +def-blkdeviotune.write_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if ((def-blkdeviotune.total_bytes_sec def-blkdeviotune.read_bytes_sec) +|| (def-blkdeviotune.total_bytes_sec def-blkdeviotune.write_bytes_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(total and read/write bytes_sec cannot be set at the same time)); +goto error; +} + +if ((def-blkdeviotune.total_iops_sec def-blkdeviotune.read_iops_sec) +|| (def-blkdeviotune.total_iops_sec def-blkdeviotune.write_iops_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(total and read/write iops_sec cannot be set at the same time)); +goto error; +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node-name, BAD_CAST disk)) { dev-type = VIR_DOMAIN_DEVICE_DISK; if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node, -NULL, flags))) +NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node-name, BAD_CAST lease)) { dev-type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, -flags
[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
Implement the block I/O throttle setting and getting support to qemu driver. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c | 342 ++ src/qemu/qemu_monitor.c | 37 + src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 186 +++ src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 165 src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 772 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5f4a18d..0a4eaa9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -94,6 +94,8 @@ #define QEMU_NB_MEM_PARAM 3 +#define QEMU_NB_BLKIOTHROTTLE_PARAM 6 + #if HAVE_LINUX_KVM_H # include linux/kvm.h #endif @@ -10775,6 +10777,344 @@ cleanup: return ret; } +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +virDomainDefPtr persistentDef = NULL; +virDomainBlockIoTuneInfo info; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; +int i; +bool isActive; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +memset(info, 0, sizeof(info)); + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +isActive = virDomainObjIsActive(vm); + +if (flags == VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +flags = VIR_DOMAIN_AFFECT_LIVE; +else +flags = VIR_DOMAIN_AFFECT_CONFIG; +} + +if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(domain is not running)); +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!vm-persistent) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(cannot change persistent config of a transient domain)); +goto endjob; +} +if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) +goto endjob; +} + +for (i = 0; i nparams; i++) { +virTypedParameterPtr param = params[i]; + +if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + +info.total_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { +info.read_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { +info.write_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { +info.total_iops_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { +info.read_iops_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { +info.write_iops_sec = params[i].value.ul; +} else { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(Unrecognized parameter)); +goto endjob; +} +} + +if ((info.total_bytes_sec info.read_bytes_sec) || +(info.total_bytes_sec info.write_bytes_sec)) { +qemuReportError(VIR_ERR_INVALID_ARG, %s, +_(total and read/write of bytes_sec cannot be set at the same time)); +goto endjob; +} + +if ((info.total_iops_sec info.read_iops_sec) || +(info.total_iops_sec info.write_iops_sec)) { +qemuReportError(VIR_ERR_INVALID_ARG, %s, +_(total and read/write of iops_sec cannot be set at the same time)); +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, 1); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +int idx
[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Support Block I/O Throttle setting and query to remote driver. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- daemon/remote.c | 109 ++ src/remote/remote_driver.c | 96 + src/remote/remote_protocol.x | 27 ++- src/remote_protocol-structs | 24 + 4 files changed, 255 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index aa3f768..227d36e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1886,6 +1886,115 @@ cleanup: return rv; } +static int +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_set_block_io_throttle_args *args) +{ +virDomainPtr dom = NULL; +int rv = -1; +virTypedParameterPtr params = NULL; +int nparams; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if ((params = remoteDeserializeTypedParameters(args-params.params_val, + args-params.params_len, + REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX, + nparams)) == NULL) +goto cleanup; + +rv = virDomainSetBlockIoTune(dom, args-disk, params, nparams, args-flags); + +if (rv 0) +goto cleanup; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +int rv = -1; +int i; +virTypedParameterPtr params; +int nparams = args-nparams; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (nparams REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large)); +goto cleanup; +} + +if (VIR_ALLOC_N(params, nparams) 0) { +virReportOOMError(); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (virDomainGetBlockIoTune(dom, args-disk, params, nparams, args-flags) 0) +goto cleanup; + +/* In this case, we need to send back the number of parameters + * supported + */ +if (args-nparams == 0) { +ret-nparams = nparams; +goto success; +} + +/* Serialise the block I/O throttle. */ +if (remoteSerializeTypedParameters(params, nparams, + ret-params.params_val, + ret-params.params_len, + args-flags) 0) +goto cleanup; + +success: +rv = 0; + +cleanup: +if (rv 0) { +virNetMessageSaveError(rerr); +if (ret-params.params_val) { +for (i = 0; i nparams; i++) +VIR_FREE(ret-params.params_val[i].field); +VIR_FREE(ret-params.params_val); +} +} +if (dom) +virDomainFree(dom); +return rv; +} /*-*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 94fd3e7..fa2d2c7 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2178,6 +2178,100 @@ done: return rv; } +static int remoteDomainSetBlockIoTune(virDomainPtr domain, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ +int rv = -1; +remote_domain_set_block_io_throttle_args args; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv
[libvirt] [PATCH 8/8] Add tests for blkdeviotune
Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- .../qemuxml2argv-blkdeviotune.args |4 ++ .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 36 tests/qemuxml2argvtest.c |1 + tests/qemuxml2xmltest.c|1 + 4 files changed, 42 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args new file mode 100644 index 000..6f34698 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor unix:/tmp/test-monitor, \ +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml new file mode 100644 index 000..1fceef8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml @@ -0,0 +1,36 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219100/memory + currentMemory219100/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' snapshot='internal' + driver name='qemu' type='qcow2' cache='none'/ + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + iotune +total_bytes_sec5000/total_bytes_sec +total_iops_sec6000/total_iops_sec + address type='drive' controller='0' bus='0' unit='0'/ +/disk +disk type='block' device='cdrom' snapshot='no' + driver name='qemu' type='raw'/ + source dev='/dev/HostVG/QEMUGuest2'/ + target dev='hdc' bus='ide'/ + readonly/ + address type='drive' controller='0' bus='1' unit='0'/ +/disk +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d9a6e8d..1ebb950 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -585,6 +585,7 @@ mymain(void) DO_TEST(blkiotune, false, QEMU_CAPS_NAME); DO_TEST(cputune, false, QEMU_CAPS_NAME); DO_TEST(numatune-memory, false, NONE); +DO_TEST(blkdeviotune, false, QEMU_CAPS_NAME); DO_TEST(multifunction-pci-device, false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3f37520..2e6b5c7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -191,6 +191,7 @@ mymain(void) DO_TEST(event_idx); DO_TEST(usb-redir); +DO_TEST(blkdeviotune); /* These tests generate different XML */ DO_TEST_DIFFERENT(balloon-device-auto); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
Support virsh command blkdeviotune. Can set or query a block disk I/O throttle setting. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- tools/virsh.c | 240 +++ tools/virsh.pod | 23 + 2 files changed, 263 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 83dc3c7..9eec68e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6074,6 +6074,245 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd) } /* + * blkdeviotune command + */ +static const vshCmdInfo info_blkdeviotune[] = { +{help, N_(Set or query a block disk I/O throttle setting.)}, +{desc, N_(Set or query a block disk I/O throttle setting.\n \ +To query the block disk I/O throttle setting use the following \ + command: \n\n \ +virsh # blkdeviotune domain device)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkdeviotune[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limit in bytes per second)}, +{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limit in bytes per second)}, +{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limit in bytes per second)}, +{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total I/O operations limit per second)}, +{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read I/O operations limit per second)}, +{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write I/O operations limit per second)}, +{config, VSH_OT_BOOL, 0, N_(affect next boot)}, +{live, VSH_OT_BOOL, 0, N_(affect running domain)}, +{current, VSH_OT_BOOL, 0, N_(affect current domain)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +unsigned long long total_bytes_sec = 0, read_bytes_sec = 0, write_bytes_sec = 0; +unsigned long long total_iops_sec = 0, read_iops_sec = 0, write_iops_sec = 0; +int nparams = 0; +virTypedParameterPtr params = NULL, temp = NULL; +unsigned int flags = 0, i = 0; +int rv = 0; +int current = vshCommandOptBool(cmd, current); +int config = vshCommandOptBool(cmd, config); +int live = vshCommandOptBool(cmd, live); + +if (current) { +if (live || config) { +vshError(ctl, %s, _(--current must be specified exclusively)); +return false; +} +flags = VIR_DOMAIN_AFFECT_CURRENT; +} else { +if (config) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; +} + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec, total_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, read_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec, write_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, total_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, read_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_iops_sec, write_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if (nparams == 0) { + +if ((virDomainGetBlockIoTune(dom, disk, NULL, nparams, flags)) != 0) { +vshError(ctl, %s, + _(Unable to get number of block I/O throttle parameters)); +goto out; +} + +if (nparams == 0) { +virDomainFree(dom); +return true; +} + +params = vshCalloc(ctl, nparams, sizeof(*params
[libvirt] [PATCH 0/8 v5] Summary on block IO throttle
Changes since V3 - Use virTypedParameterPtr instead of specific struct in libvirt pulic API. - Relevant changes to remote driver, qemu driver, python support and virsh. Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotune domain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotune domain device [--total_bytes_sec number] [--read_bytes_sec number] [--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec number] [--write_iops_sec number] [--config] [--live] [--current] daemon/remote.c| 108 +++ docs/formatdomain.html.in | 31 ++ docs/schemas/domaincommon.rng | 24 ++ include/libvirt/libvirt.h.in | 70 python/generator.py|2 + python/libvirt-override-api.xml| 16 + python/libvirt-override.c | 179 +++ src/conf/domain_conf.c | 101 ++- src/conf/domain_conf.h | 12 + src/driver.h | 20 ++ src/libvirt.c | 145 + src/libvirt_public.syms|2 + src/qemu/qemu_command.c| 33 ++ src/qemu/qemu_driver.c | 338 src/qemu/qemu_monitor.c| 36 ++ src/qemu/qemu_monitor.h| 22 ++ src/qemu/qemu_monitor_json.c | 185 +++ src/qemu/qemu_monitor_json.h | 10 + src/qemu/qemu_monitor_text.c | 164 ++ src/qemu/qemu_monitor_text.h | 10 + src/remote/remote_driver.c | 96 ++ src/remote/remote_protocol.x | 26 ++- src/remote_protocol-structs| 24 ++ src/util/xml.h |2
[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 63 +++ src/driver.h | 20 ++ src/libvirt.c| 141 ++ src/libvirt_public.syms |6 ++ 4 files changed, 230 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2ab89f5..dfa8f76 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1671,6 +1671,69 @@ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* Block I/O throttling support */ + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it represents the total + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC total_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC read_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the write + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC write_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the total + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC total_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC read_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC: + * Macro for the BlockIoTune tunable weight: it repersents the write + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC write_iops_sec + +int +virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int nparams, +unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int *nparams, +unsigned int flags); + + /* * NUMA support */ diff --git a/src/driver.h b/src/driver.h index 4c14aaa..23e96d9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -740,6 +740,24 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int +(*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int +(*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + /** * _virDriver: @@ -899,6 +917,8 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; +virDrvDomainSetBlockIoTune domainSetBlockIoTune; +virDrvDomainGetBlockIoTune domainGetBlockIoTune; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..2a39958 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17149,3 +17149,144 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name (or device shothand name) + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: An OR'ed set of virDomainModificationImpact + * + * Change all or a subset of the per-device block IO tunables. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the target dev='...' + * attribute within //domain/devices
[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 69 src/driver.h | 18 + src/libvirt.c| 142 ++ src/libvirt_public.syms |2 + 4 files changed, 231 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2ab89f5..f4988c4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1671,6 +1671,75 @@ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* Block I/O throttling support */ + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it represents the total + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC total_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC read_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the write + * bytes per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC write_bytes_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the total + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC total_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC read_iops_sec + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC: + * Macro for the BlockIoTune tunable weight: it repersents the write + * I/O operations per second permitted through a block device, as a ullong. + */ + +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC write_iops_sec + +int +virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int nparams, +unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int *nparams, +unsigned int flags); + + /* * NUMA support */ diff --git a/src/driver.h b/src/driver.h index 4c14aaa..6ce3efc 100644 --- a/src/driver.h +++ b/src/driver.h @@ -740,6 +740,24 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int +(*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int +(*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + /** * _virDriver: diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..32ad5db 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17149,3 +17149,145 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: An OR'ed set of virDomainModificationImpact + * + * Change all or a subset of the per-device block IO tunables. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the target dev='...' + * attribute within //domain/devices/disk. (For example, xvda). + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virTypedParameterPtr params, +int nparams, +unsigned int flags
[libvirt] [PATCH 6/8] Doc: Add description and validation for blkdeviotune
Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- docs/formatdomain.html.in | 31 +++ docs/schemas/domaincommon.rng | 24 2 files changed, 55 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cbad196..733062d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -893,6 +893,14 @@ lt;driver name=tap type=aio cache=default/gt; lt;source file='/var/lib/xen/images/fv0'/ startupPolicy='optional'gt; lt;target dev='hda' bus='ide'/gt; + lt;iotunegt; +lt;total_bytes_secgt;nlt;/total_bytes_secgt; +lt;read_bytes_secgt;nlt;/read_bytes_secgt; +lt;write_bytes_secgt;nlt;/write_bytes_secgt; +lt;total_iops_secgt;nlt;/total_iops_secgt; +lt;read_bytes_secgt;nlt;/read_bytes_secgt; +lt;write_bytes_secgt;nlt;/write_bytes_secgt; + lt;/iotunegt; lt;boot order='2'/gt; lt;encryption type='...'gt; ... @@ -1010,6 +1018,29 @@ span class=sinceSince 0.0.3; codebus/code attribute since 0.4.3; usb attribute value since after 0.4.4; sata attribute value since 0.9.7/span/dd + dtcodeiotune/code/dt + ddThe optional codeiotune/code element provides the ability +to set or get block I/O throttling for the device. Block I/O +throtting be implemented by qemu, is specified per-disk and can +vary across multiple disks./dd + dtcodetotal_bytes_sec/code/dt + ddThe optinal codetotal_bytes_sec/code element is the total throughput +limit in bytes per second./dd + dtcoderead_bytes_sec/code/dt + ddThe optinal coderead_bytes_sec/code element is the read throughput +limit in bytes per second./dd + dtcodewrite_bytes_sec/code/dt + ddThe optinal codewrite_bytes_sec/code element is the write throughput +limit in bytes per second./dd + dtcodetotal_iops_sec/code/dt + ddThe optional codetotal_iops_sec/code element is the total I/O operations +per second./dd + dtcoderead_iops_sec/code/dt + ddThe optional coderead_iops_sec/code element is the read I/O operations +per second./dd + dtcodewrite_iops_sec/code/dt + ddThe optional codewrite_iops_sec/code element is the write I/O operations +per second./dd dtcodedriver/code/dt dd The optional driver element allows specifying further details diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b6f858e..c6873a0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -698,6 +698,30 @@ optional ref name=snapshot/ /optional + optional +element name=iotune + choice +element name=total_bytes_sec + ref name=unsignedLongLong +/element +element name=read_bytes_sec + ref name=unsignedLongLong +/element +element name=write_bytes_sec + ref name=unsignedLongLong +/element +element name=total_iops_sec + ref name=unsignedLongLong +/element +element name=read_iops_sec + ref name=unsignedLongLong +/element +element name=write_iopw_sec + ref name=unsignedLongLong +/emement + /choice +/element + /optional choice group attribute name=type -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8 v4] Summary on block IO throttle
Changes since V3 - Use virTypedParameterPtr instead of specific struct in libvirt pulic API. - Relevant changes to remote driver, qemu driver, python support and virsh. Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotune domain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotune domain device [--total_bytes_sec number] [--read_bytes_sec number] [--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec number] [--write_iops_sec number] [--config] [--live] [--current] daemon/remote.c| 108 +++ docs/formatdomain.html.in | 31 ++ docs/schemas/domaincommon.rng | 24 ++ include/libvirt/libvirt.h.in | 70 python/generator.py|2 + python/libvirt-override-api.xml| 16 + python/libvirt-override.c | 179 +++ src/conf/domain_conf.c | 101 ++- src/conf/domain_conf.h | 12 + src/driver.h | 20 ++ src/libvirt.c | 145 + src/libvirt_public.syms|2 + src/qemu/qemu_command.c| 33 ++ src/qemu/qemu_driver.c | 338 src/qemu/qemu_monitor.c| 36 ++ src/qemu/qemu_monitor.h| 22 ++ src/qemu/qemu_monitor_json.c | 185 +++ src/qemu/qemu_monitor_json.h | 10 + src/qemu/qemu_monitor_text.c | 164 ++ src/qemu/qemu_monitor_text.h | 10 + src/remote/remote_driver.c | 96 ++ src/remote/remote_protocol.x | 26 ++- src/remote_protocol-structs| 24 ++ src/util/xml.h |2
[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 101 +- src/conf/domain_conf.h | 12 ++ src/qemu/qemu_command.c | 33 +++ src/util/xml.h |2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..a157b80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child-next; } +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +if (virXPathULongLong(string(./devices/disk/iotune/total_bytes_sec), + ctxt, def-blkdeviotune.total_bytes_sec) 0) { +def-blkdeviotune.total_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_bytes_sec), + ctxt, def-blkdeviotune.read_bytes_sec) 0) { +def-blkdeviotune.read_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_bytes_sec), + ctxt, def-blkdeviotune.write_bytes_sec) 0) { +def-blkdeviotune.write_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + + if (virXPathULongLong(string(./devices/disk/iotune/total_iops_sec), + ctxt, def-blkdeviotune.total_iops_sec) 0) { +def-blkdeviotune.total_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_iops_sec), + ctxt, def-blkdeviotune.read_iops_sec) 0) { +def-blkdeviotune.read_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_iops_sec), + ctxt, def-blkdeviotune.write_iops_sec) 0) { +def-blkdeviotune.write_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if ((def-blkdeviotune.total_bytes_sec def-blkdeviotune.read_bytes_sec) +|| (def-blkdeviotune.total_bytes_sec def-blkdeviotune.write_bytes_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(total and read/write bytes_sec cannot be set at the same time)); +goto error; +} + +if ((def-blkdeviotune.total_iops_sec def-blkdeviotune.read_iops_sec) +|| (def-blkdeviotune.total_iops_sec def-blkdeviotune.write_iops_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(total and read/write iops_sec cannot be set at the same time)); +goto error; +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node-name, BAD_CAST disk)) { dev-type = VIR_DOMAIN_DEVICE_DISK; if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node, -NULL, flags))) +NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node-name, BAD_CAST lease)) { dev-type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, -flags
[libvirt] [PATCH 8/8] Add tests for blkdeviotune
Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- .../qemuxml2argv-blkdeviotune.args |5 +++ .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 37 tests/qemuxml2argvtest.c |1 + tests/qemuxml2xmltest.c|1 + 4 files changed, 44 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args new file mode 100644 index 000..9db8aff --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor unix:/tmp/test-monitor, \ +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none -usb + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml new file mode 100644 index 000..0cabb90 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml @@ -0,0 +1,37 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219100/memory + currentMemory219100/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' snapshot='internal' + driver name='qemu' type='qcow2' cache='none'/ + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + iotune +total_bytes_sec5000/total_bytes_sec +total_iops_sec6000/total_iops_sec + address type='drive' controller='0' bus='0' unit='0'/ +/disk +disk type='block' device='cdrom' snapshot='no' + driver name='qemu' type='raw'/ + source dev='/dev/HostVG/QEMUGuest2'/ + target dev='hdc' bus='ide'/ + readonly/ + address type='drive' controller='0' bus='1' unit='0'/ +/disk +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d9a6e8d..1ebb950 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -585,6 +585,7 @@ mymain(void) DO_TEST(blkiotune, false, QEMU_CAPS_NAME); DO_TEST(cputune, false, QEMU_CAPS_NAME); DO_TEST(numatune-memory, false, NONE); +DO_TEST(blkdeviotune, false, QEMU_CAPS_NAME); DO_TEST(multifunction-pci-device, false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3f37520..2e6b5c7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -191,6 +191,7 @@ mymain(void) DO_TEST(event_idx); DO_TEST(usb-redir); +DO_TEST(blkdeviotune); /* These tests generate different XML */ DO_TEST_DIFFERENT(balloon-device-auto); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Support Block I/O Throttle setting and query to remote driver. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- daemon/remote.c | 109 ++ src/remote/remote_driver.c | 96 + src/remote/remote_protocol.x | 26 ++- src/remote_protocol-structs | 24 + 4 files changed, 254 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index aa3f768..227d36e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1886,6 +1886,115 @@ cleanup: return rv; } +static int +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_set_block_io_throttle_args *args) +{ +virDomainPtr dom = NULL; +int rv = -1; +virTypedParameterPtr params = NULL; +int nparams; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if ((params = remoteDeserializeTypedParameters(args-params.params_val, + args-params.params_len, + REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX, + nparams)) == NULL) +goto cleanup; + +rv = virDomainSetBlockIoTune(dom, args-disk, params, nparams, args-flags); + +if (rv 0) +goto cleanup; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +int rv = -1; +int i; +virTypedParameterPtr params; +int nparams = args-nparams; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (nparams REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large)); +goto cleanup; +} + +if (VIR_ALLOC_N(params, nparams) 0) { +virReportOOMError(); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (virDomainGetBlockIoTune(dom, args-disk, params, nparams, args-flags) 0) +goto cleanup; + +/* In this case, we need to send back the number of parameters + * supported + */ +if (args-nparams == 0) { +ret-nparams = nparams; +goto success; +} + +/* Serialise the block I/O throttle. */ +if (remoteSerializeTypedParameters(params, nparams, + ret-params.params_val, + ret-params.params_len, + args-flags) 0) +goto cleanup; + +success: +rv = 0; + +cleanup: +if (rv 0) { +virNetMessageSaveError(rerr); +if (ret-params.params_val) { +for (i = 0; i nparams; i++) +VIR_FREE(ret-params.params_val[i].field); +VIR_FREE(ret-params.params_val); +} +} +if (dom) +virDomainFree(dom); +return rv; +} /*-*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 94fd3e7..fa2d2c7 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2178,6 +2178,100 @@ done: return rv; } +static int remoteDomainSetBlockIoTune(virDomainPtr domain, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ +int rv = -1; +remote_domain_set_block_io_throttle_args args; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv
[libvirt] [PATCH 7/8] Support virDomain{Set, Get}BlockIoThrottle in the python API
Python support for both setting and getting block I/O throttle. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- python/generator.py |2 + python/libvirt-override-api.xml | 16 python/libvirt-override.c | 178 +++ 3 files changed, 196 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 71afdb7..88c52b9 100755 --- a/python/generator.py +++ b/python/generator.py @@ -414,6 +414,8 @@ skip_impl = ( 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', 'virDomainBlockStatsFlags', +'virDomainSetBlockIoTune', +'virDomainGetBlockIoTune', ) qemu_skip_impl = ( diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..a05da3c 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -375,5 +375,21 @@ arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/ return type='unsigned long' info='current max migration speed, or None in case of error'/ /function +function name='virDomainSetBlockIoTune' file='python' + infoChange the I/O throttle for a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='params' type='virTypedParameterPtr' info='Pointer to blkio throttle params object'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='int' info='0 in case of success, -1 in case of failure'/ +/function +function name='virDomainGetBlockIoTune' file='python' + infoGet the I/O throttle a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='params' type='virTypedParameterPtr' info='Pointer to blkio throttle params object'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='int' info='0 in case of success, -1 in case of failure'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1759bae..be76d87 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3195,6 +3195,182 @@ LIBVIRT_END_ALLOW_THREADS; return ret; } +static PyObject * +libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain, *pyinfo; +const char *disk; +unsigned int flags; +virTypedParameterPtr params; +int nparams = 0, i; +int c_ret; + +if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainSetBlockIoTune, + pyobj_domain, disk, pyinfo, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainGetBlockIoTune(domain, disk, NULL, nparams, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) +return VIR_PY_INT_FAIL; + +if ((params = malloc(sizeof(*params)*nparams)) == NULL) +return VIR_PY_INT_FAIL; + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainGetBlockIoTune(domain, disk, params, nparams, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) { +free(params); +return VIR_PY_INT_FAIL; +} + +/* convert to a Python tuple of long objects */ +for (i = 0; i nparams; i++) { +PyObject *key, *val; +key = libvirt_constcharPtrWrap(params[i].field); +val = PyDict_GetItem(pyinfo, key); +Py_DECREF(key); + +if (val == NULL) +continue; + +switch (params[i].type) { +case VIR_TYPED_PARAM_INT: +params[i].value.i = (int)PyInt_AS_LONG(val); +break; + +case VIR_TYPED_PARAM_UINT: +params[i].value.ui = (unsigned int)PyInt_AS_LONG(val); +break; + +case VIR_TYPED_PARAM_LLONG: +params[i].value.l = (long long)PyLong_AsLongLong(val); +break; + +case VIR_TYPED_PARAM_ULLONG: +params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val); +break; + +case VIR_TYPED_PARAM_DOUBLE: +params[i].value.d = (double)PyFloat_AsDouble(val); +break; + +case VIR_TYPED_PARAM_BOOLEAN: +{ +PyObject *hacktrue = PyBool_FromLong(1); +params[i].value.b = hacktrue == val ? 1: 0; +Py_DECREF(hacktrue); +} +break; + +default: +free(params); +return VIR_PY_INT_FAIL; +} +} + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainSetMemoryParameters(domain, params, nparams, flags
[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
This patch implement the block I/O throttle setting and getting support to qemu driver. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c | 340 ++ src/qemu/qemu_monitor.c | 37 + src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 185 +++ src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 164 src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 768 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5f4a18d..740d2a2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10775,6 +10775,344 @@ cleanup: return ret; } +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +virDomainDefPtr persistentDef = NULL; +virDomainBlockIoTuneInfo info; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; +int i; +bool isActive; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +memset(info, 0, sizeof(info)); + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +isActive = virDomainObjIsActive(vm); + +if (flags == VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +flags = VIR_DOMAIN_AFFECT_LIVE; +else +flags = VIR_DOMAIN_AFFECT_CONFIG; +} + +if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(domain is not running)); +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!vm-persistent) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(cannot change persistent config of a transient domain)); +goto endjob; +} +if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) +goto endjob; +} + +for (i = 0; i nparams; i++) { +virTypedParameterPtr param = params[i]; + +if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + +info.total_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { +info.read_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { +info.write_bytes_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { +info.total_iops_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { +info.read_iops_sec = params[i].value.ul; +} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { +info.write_iops_sec = params[i].value.ul; +} else { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(Unrecognized parameter)); +goto endjob; +} +} + +if ((info.total_bytes_sec info.read_bytes_sec) || +(info.total_bytes_sec info.write_bytes_sec)) { +qemuReportError(VIR_ERR_INVALID_ARG, %s, +_(total and read/write of bytes_sec cannot be set at the same time)); +goto endjob; +} + +if ((info.total_iops_sec info.read_iops_sec) || +(info.total_iops_sec info.write_iops_sec)) { +qemuReportError(VIR_ERR_INVALID_ARG, %s, +_(total and read/write of iops_sec cannot be set at the same time)); +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, 1); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +int idx = virDomainDiskIndexByName(vm-def, disk, true); +if (i 0) +goto endjob; +persistentDef-disks[idx]-blkdeviotune.total_bytes_sec
[libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
Support virsh command blkdeviotune. Can set or query a block disk I/O throttle setting. Signed-off-by: Lei Li li...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- tools/virsh.c | 240 +++ tools/virsh.pod | 23 + 2 files changed, 263 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 83dc3c7..9eec68e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6074,6 +6074,245 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd) } /* + * blkdeviotune command + */ +static const vshCmdInfo info_blkdeviotune[] = { +{help, N_(Set or query a block disk I/O throttle setting.)}, +{desc, N_(Set or query a block disk I/O throttle setting.\n \ +To query the block disk I/O throttle setting use the following \ + command: \n\n \ +virsh # blkdeviotune domain device)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkdeviotune[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limit in bytes per second)}, +{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limit in bytes per second)}, +{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limit in bytes per second)}, +{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total I/O operations limit per second)}, +{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read I/O operations limit per second)}, +{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write I/O operations limit per second)}, +{config, VSH_OT_BOOL, 0, N_(affect next boot)}, +{live, VSH_OT_BOOL, 0, N_(affect running domain)}, +{current, VSH_OT_BOOL, 0, N_(affect current domain)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +unsigned long long total_bytes_sec = 0, read_bytes_sec = 0, write_bytes_sec = 0; +unsigned long long total_iops_sec = 0, read_iops_sec = 0, write_iops_sec = 0; +int nparams = 0; +virTypedParameterPtr params = NULL, temp = NULL; +unsigned int flags = 0, i = 0; +int rv = 0; +int current = vshCommandOptBool(cmd, current); +int config = vshCommandOptBool(cmd, config); +int live = vshCommandOptBool(cmd, live); + +if (current) { +if (live || config) { +vshError(ctl, %s, _(--current must be specified exclusively)); +return false; +} +flags = VIR_DOMAIN_AFFECT_CURRENT; +} else { +if (config) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; +} + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec, total_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, read_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec, write_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, total_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, read_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_iops_sec, write_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +nparams++; +} + +if (nparams == 0) { + +if ((virDomainGetBlockIoTune(dom, disk, NULL, nparams, flags)) != 0) { +vshError(ctl, %s, + _(Unable to get number of block I/O throttle parameters)); +goto out; +} + +if (nparams == 0) { +virDomainFree(dom); +return true; +} + +params = vshCalloc(ctl, nparams, sizeof(*params
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/15/2011 12:46 PM, Eric Blake wrote: On 11/14/2011 09:33 PM, Lei Li wrote: This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name Hmm. Fully-qualified name here, + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: An OR'ed set of virDomainModificationImpact + * + * Change all or a subset of the per-device block IO tunables. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding thetarget dev='...' + * attribute within //domain/devices/disk. (For example, xvda). but device shorthand here. We probably ought to accept both forms, just as we do with virDomainBlockStats, as of commit 89b6284f (oh, and that means that I ought to update virDomainBlockStats to document the same thing) - which means this can be a separate cleanup to make libvirt.c docs consistent as well as teaching the new API to use the domain_conf.c virDomainDiskIndexByName method to allow both forms. I can get to that after finishing my review of this series, if you don't beat me. Yes, both fully-qualified and shorthand name can all work. + * Get all block IO tunable parameters for a given device. On input, + * @nparams gives the size of the @params array; on output, @nparams + * gives how many slots were filled with parameter information, which + * might be less but will not exceed the input value. + * + * As a special case, calling with @params as NULL and @nparams as 0 on + * input will cause @nparams on output to contain the number of parameters + * supported by the hypervisor. The caller should then allocate @params + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API + * agiain. Trailing space (running 'make syntax-check' will gripe). Thanks..:) s/agiain/again/ Also, probably worth mentioning that the value of @nparams may be disk-specific, so the user is best off reprobing an appropriate nparams for each disk rather than assuming the parameters from 1 disk apply to all others. The QEMU Block I/O Throttling support these whole 6 parameters, so I think display all the fields might make sense. +++ b/src/libvirt_public.syms @@ -496,6 +496,8 @@ LIBVIRT_0.9.7 { virDomainSnapshotGetParent; virDomainSnapshotListChildrenNames; virDomainSnapshotNumChildren; +virDomainSetBlockIoTune; +virDomainGetBlockIoTune; } LIBVIRT_0.9.5; This needs to be in a new section LIBVIRT_0.9.8 based off of LIBVIRT_0.9.5 (we aren't changing 0.9.7 at this point). The problems I mentioned are minor enough that I don't mind fixing them prior to pushing, if the rest of the series looks sane; but if later review finds more problems later in the series, we may need a v5 series instead. I'll see about reviewing the rest of the series this week (I've also got quite a few nwfilter patches backed up in my todo queue). OK. :) Thank you. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/8 v3] Summary on block IO throttle
On 11/11/2011 05:50 AM, Adam Litke wrote: Hi Lei, I've tested this and found that it works well except for one small thing. When the domain is running and I change one of the throttle settings using the --config option in virsh, the on-disk domain xml file is properly updated, but the command 'virsh dumpxml' returns the old version. Is this how it is supposed to work? Hi Adam, I tried to test this case. I found that the command 'virsh dumpxml' will return the updated version after restart or just destroy the domain. :) Otherwise, it seems to work as advertised (based on my sniff testing). Tested-by: Adam Litkea...@us.ibm.com On Thu, Nov 10, 2011 at 04:32:50AM +0800, Lei HH Li wrote: Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottledomain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] [--config] [--live] [--current] daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py |2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c| 101 src/conf/domain_conf.h| 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms |2 src/qemu/qemu_command.c | 33 ++ src/qemu/qemu_driver.c| 217 ++ src/qemu/qemu_monitor.c
[libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
Support virsh command blkdeviotune. Can set or query a block disk I/O throttle setting. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- tools/virsh.c | 146 +++ tools/virsh.pod | 23 + 2 files changed, 169 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 83dc3c7..df2b399 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6023,6 +6023,151 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * blkdeviotune command + */ +static const vshCmdInfo info_blkdeviotune[] = { +{help, N_(Set or query a block disk I/O throttle setting.)}, +{desc, N_(Set or query a block disk I/O throttle setting.\n \ +To query the block disk I/O throttle setting use the following \ + command: \n\n \ +virsh # blkdeviotune domain device)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkdeviotune[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limit in bytes per second/s)}, +{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limit in bytes per second/s)}, +{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limit in bytes per second/s)}, +{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total I/O operations limit per second/s)}, +{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read I/O operations limit per second/s)}, +{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write I/O operations limit per second/s)}, +{config, VSH_OT_BOOL, 0, N_(affect next boot)}, +{live, VSH_OT_BOOL, 0, N_(affect running domain)}, +{current, VSH_OT_BOOL, 0, N_(affect current domain)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +virDomainBlockIoTuneInfo info; +virDomainBlockIoTuneInfo reply; +unsigned int flags = 0; +int ret = -1; +int rv = 0; +int set = 0; +int current = vshCommandOptBool(cmd, current); +int config = vshCommandOptBool(cmd, config); +int live = vshCommandOptBool(cmd, live); + +if (current) { +if (live || config) { +vshError(ctl, %s, _(--current must be specified exclusively)); +return false; +} +flags = VIR_DOMAIN_AFFECT_CURRENT; +} else { +if (config) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; +} + +memset(info, 0, sizeof(info)); + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec, info.total_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, info.read_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec, info.write_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, info.total_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, info.read_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_iops_sec, info.write_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if (!set) { + +ret = virDomainGetBlockIoTune(dom, disk, reply, flags); + +if (ret != 0) +goto out; + +vshPrint(ctl, %-15s %llu\n, _(total_bytes_sec:), reply.total_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(read_bytes_sec:), reply.read_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(write_bytes_sec:), reply.write_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(total_iops_sec:), reply.total_iops_sec); +vshPrint(ctl, %-15s %llu\n, _(read_iops_sec
[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c| 115 ++ src/libvirt_public.syms |2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { +unsigned long long total_bytes_sec; +unsigned long long read_bytes_sec; +unsigned long long write_bytes_sec; +unsigned long long total_iops_sec; +unsigned long long read_iops_sec; +unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; + +int +virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr info, +unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr reply, +unsigned int flags); + /* * NUMA support diff --git a/src/driver.h b/src/driver.h index 4c14aaa..9628ad7 100644 --- a/src/driver.h +++ b/src/driver.h @@ -741,6 +741,23 @@ typedef int unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int +(*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags); + +typedef int +(*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags); + + /** * _virDriver: * @@ -899,6 +916,8 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; +virDrvDomainSetBlockIoTune domainSetBlockIoTune; +virDrvDomainGetBlockIoTune domainGetBlockIoTune; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index b0d1e01..79ac84d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17083,3 +17083,118 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @info: Specify block I/O limits in bytes + * @flags: An OR'ed set of virDomainModificationImpact + * + * This function is mainly to enable Block I/O throttling function in libvirt. + * It is used to change the block I/O throttling setting for specified domain. + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr info, +unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(dom, disk=%p, info=%p, flags=%x, + disk, info, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN (dom)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +conn = dom-conn; + +if (dom-conn-flags VIR_CONNECT_RO) { +virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (!disk) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +if (!info) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +if (conn-driver-domainSetBlockIoTune) { +int ret; +ret = conn-driver-domainSetBlockIoTune(dom, disk, info, flags); +if (ret 0) +goto error; +return ret; +} + +virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(dom-conn); +return -1; +} + +/** + * virDomainGetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @reply: Specify block I/O info in bytes + * @flags: An OR'ed set of virDomainModificationImpact + * + * This function
[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
This patch implement the blk io throttle setting and getting support to qemu driver. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c | 216 ++ src/qemu/qemu_monitor.c | 36 +++ src/qemu/qemu_monitor.h | 10 ++ src/qemu/qemu_monitor_json.c | 191 + src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 152 + src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 625 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db2ac0d..7ca6719 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10759,6 +10759,220 @@ cleanup: return ret; } +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +virDomainDefPtr persistentDef = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; +bool isActive; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +isActive = virDomainObjIsActive(vm); + +if (flags == VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +flags = VIR_DOMAIN_AFFECT_LIVE; +else +flags = VIR_DOMAIN_AFFECT_CONFIG; +} + +if ((info-total_bytes_sec info-read_bytes_sec) || +(info-total_bytes_sec info-write_bytes_sec)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(bps and bps_rd/bps_wr cannot be used at the same time)); +goto endjob; +} + +if ((info-total_iops_sec info-read_iops_sec) || +(info-total_iops_sec info-write_iops_sec)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(iops and iops_rd/iops_wr cannot be used at the same time)); +goto endjob; +} + +if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(domain is not running)); +goto endjob; +} + + if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!vm-persistent) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(cannot change persistent config of a transient domain)); +goto endjob; +} +if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, 1); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +int i = virDomainDiskIndexByName(vm-def, disk, true); +if (i 0) +goto endjob; +persistentDef-disks[i]-blkdeviotune.total_bytes_sec = info-total_bytes_sec; +persistentDef-disks[i]-blkdeviotune.read_bytes_sec = info-read_bytes_sec; +persistentDef-disks[i]-blkdeviotune.write_bytes_sec = info-write_bytes_sec; +persistentDef-disks[i]-blkdeviotune.total_iops_sec = info-total_iops_sec; +persistentDef-disks[i]-blkdeviotune.read_iops_sec = info-read_iops_sec; +persistentDef-disks[i]-blkdeviotune.write_iops_sec = info-write_iops_sec; +persistentDef-disks[i]-blkdeviotune.mark = 1; +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +ret = virDomainSaveConfig(driver-configDir, persistentDef); +if (ret 0) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, + _(Write to config file failed)); +goto endjob; +} +} + +endjob: +if (qemuDomainObjEndJob(driver, vm) == 0) +vm = NULL; + +cleanup: +VIR_FREE(device); +if (vm) +virDomainObjUnlock(vm); +qemuDriverUnlock(driver); +return ret; +} + +static int +qemuDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr reply
[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 101 +- src/conf/domain_conf.h | 12 ++ src/qemu/qemu_command.c | 33 +++ src/util/xml.h |2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..564914e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child-next; } +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +if (virXPathULongLong(string(./devices/disk/iotune/total_bytes_sec), + ctxt, def-blkdeviotune.total_bytes_sec) 0) { +def-blkdeviotune.total_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_bytes_sec), + ctxt, def-blkdeviotune.read_bytes_sec) 0) { +def-blkdeviotune.read_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_bytes_sec), + ctxt, def-blkdeviotune.write_bytes_sec) 0) { +def-blkdeviotune.write_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + + if (virXPathULongLong(string(./devices/disk/iotune/total_iops_sec), + ctxt, def-blkdeviotune.total_iops_sec) 0) { +def-blkdeviotune.total_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_iops_sec), + ctxt, def-blkdeviotune.read_iops_sec) 0) { +def-blkdeviotune.read_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_iops_sec), + ctxt, def-blkdeviotune.write_iops_sec) 0) { +def-blkdeviotune.write_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if ((def-blkdeviotune.total_bytes_sec def-blkdeviotune.read_bytes_sec) +|| (def-blkdeviotune.total_bytes_sec def-blkdeviotune.write_bytes_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(bps and bps_rd/bps_wr cannot be set at the same time)); +goto error; +} + +if ((def-blkdeviotune.total_iops_sec def-blkdeviotune.read_iops_sec) +|| (def-blkdeviotune.total_iops_sec def-blkdeviotune.write_iops_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(iops and iops_rd/iops_wr cannot be set at the same time)); +goto error; +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node-name, BAD_CAST disk)) { dev-type = VIR_DOMAIN_DEVICE_DISK; if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node, -NULL, flags))) +NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node-name, BAD_CAST lease)) { dev-type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, -flags
[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Support Block I/O Throttle setting and query to remote driver. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- daemon/remote.c | 87 ++ src/remote/remote_driver.c | 80 ++ src/remote/remote_protocol.x | 38 ++- src/remote_protocol-structs | 33 4 files changed, 237 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index bd0c3e3..070ca65 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1850,6 +1850,93 @@ cleanup: return rv; } +static int +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_set_block_io_throttle_args *args) +{ +virDomainPtr dom = NULL; +virDomainBlockIoTuneInfo tmp; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (args) { +tmp.total_bytes_sec = args-bps; +tmp.read_bytes_sec = args-bps_rd; +tmp.write_bytes_sec = args-bps_wr; +tmp.total_iops_sec = args-iops; +tmp.read_iops_sec = args-iops_rd; +tmp.write_iops_sec = args-iops_wr; +} + +rv = virDomainSetBlockIoTune(dom, args-disk, tmp, args-flags); + +if (rv 0) +goto cleanup; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +virDomainBlockIoTuneInfo reply; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +rv = virDomainGetBlockIoTune(dom, args-disk, reply, args-flags); + +if (rv 0) { +ret-found = 0; +goto cleanup; +} + +ret-bps = reply.total_bytes_sec; +ret-bps_rd = reply.read_bytes_sec; +ret-bps_wr = reply.write_bytes_sec; +ret-iops= reply.total_iops_sec; +ret-iops_rd = reply.read_iops_sec; +ret-iops_wr = reply.write_iops_sec; +ret-found = 1; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} /*-*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f3b8ad5..0900231 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2154,6 +2154,84 @@ done: return rv; } +static int remoteDomainSetBlockIoTune(virDomainPtr domain, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ +int rv = -1; +remote_domain_set_block_io_throttle_args args; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv); + +memset(args, 0, sizeof(args)); + +make_nonnull_domain(args.dom, domain); +args.disk = (char *)disk; +args.bps = info-total_bytes_sec; +args.bps_rd = info-read_bytes_sec; +args.bps_wr = info-write_bytes_sec; +args.iops = info-total_iops_sec; +args.iops_rd = info-read_iops_sec; +args.iops_wr = info-write_iops_sec; +args.flags = flags; + +if (call(domain-conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_set_block_io_throttle_args, + (char *) args, + (xdrproc_t) xdr_void, + (char *) NULL) == -1) { +goto done; +} +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + +static int remoteDomainGetBlockIoTune(virDomainPtr domain, + const char *disk
[libvirt] [RFC PATCH 0/8 v3] Summary on block IO throttle
Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotune domain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotune domain device [--total_bytes_sec number] [--read_bytes_sec number] [--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec number] [--write_iops_sec number] [--config] [--live] [--current] daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py |2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c| 101 src/conf/domain_conf.h| 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms |2 src/qemu/qemu_command.c | 33 ++ src/qemu/qemu_driver.c| 217 ++ src/qemu/qemu_monitor.c | 36 ++ src/qemu/qemu_monitor.h | 10 src/qemu/qemu_monitor_json.c | 191 +++ src/qemu/qemu_monitor_json.h | 10 src/qemu/qemu_monitor_text.c | 152 src/qemu/qemu_monitor_text.h | 10 src/remote/remote_driver.c| 81 ++ src/remote/remote_protocol.x | 39 +++ src/remote_protocol-structs | 34 ++ src/util/xml.h|3 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args |4
[libvirt] [PATCH 7/8] Support virDomain{Set, Get}BlockIoThrottle in the python API
Python support for both set and get block I/O throttle. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- python/generator.py |2 + python/libvirt-override-api.xml | 16 +++ python/libvirt-override.c | 85 +++ 3 files changed, 103 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 71afdb7..88c52b9 100755 --- a/python/generator.py +++ b/python/generator.py @@ -414,6 +414,8 @@ skip_impl = ( 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', 'virDomainBlockStatsFlags', +'virDomainSetBlockIoTune', +'virDomainGetBlockIoTune', ) qemu_skip_impl = ( diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..e8d9138 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -375,5 +375,21 @@ arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/ return type='unsigned long' info='current max migration speed, or None in case of error'/ /function +function name='virDomainSetBlockIoTune' file='python' + infoChange the I/O throttle for a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='info' type='virDomainBlockIoTuneInfoPtr' info='io throttle limits'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='virDomainSetBlockIoTuneInfo' info='0 in case of operation has started, -1 in case of failure'/ +/function +function name='virDomainGetBlockIoTune' file='python' + infoGet the I/O throttle a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='reply' type='virDomainBlockIoTuneInfoPtr' info='io throttle info'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='virDomainGetBlockIoTuneInfo' info='A dictionary containing block I/O limits information.'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1759bae..d4b023a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3195,6 +3195,89 @@ LIBVIRT_END_ALLOW_THREADS; return ret; } +static PyObject * +libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain, *pyinfo; +const char *disk; +unsigned int flags; +virDomainBlockIoTuneInfo info; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainSetBlockIoTune, + pyobj_domain, disk, pyinfo, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +info.total_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, +libvirt_constcharPtrWrap(bps))); +info.read_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(bps_rd))); +info.write_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, +libvirt_constcharPtrWrap(bps_wr))); +info.total_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops))); +info.read_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops_rd))); +info.write_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops_wr))); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainSetBlockIoTune(domain, disk, info, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) { +return VIR_PY_NONE; +} + +return VIR_PY_INT_SUCCESS; +} + +static PyObject * +libvirt_virDomainGetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain; +const char *disk; +unsigned int flags; +virDomainBlockIoTuneInfo reply; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Oi:virDomainGetBlockIoTune, + pyobj_domain, disk, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainGetBlockIoTune(domain, disk, reply, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) +return VIR_PY_NONE; + +if ((ret = PyDict_New()) == NULL) +return VIR_PY_NONE
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/10/2011 04:43 AM, Eric Blake wrote: On 11/09/2011 01:32 PM, Lei Li wrote: This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c| 115 ++ src/libvirt_public.syms |2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { +unsigned long long total_bytes_sec; +unsigned long long read_bytes_sec; +unsigned long long write_bytes_sec; +unsigned long long total_iops_sec; +unsigned long long read_iops_sec; +unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure. Hi Eric, We consider using virTypeParameter as Adam suggested at first, but when I dig into the code, I found struct virTypedParameter is mainly used by memtune, blkiotune...etc, supported by cgroup mechanism, different from Block I/O throttling which is supported through qemu monitor command 'block_set_io_throttle'. Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/8 v3] Summary on block IO throttle
On 11/10/2011 12:45 PM, Zhi Yong Wu wrote: On Thu, Nov 10, 2011 at 4:32 AM, Lei Lili...@linux.vnet.ibm.com wrote: Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottledomain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] [--config] [--live] [--current] Thanks Li Lei for the remaining works. Below is only one reminder. QEMU command line options have some limitations as below: (1) global bps limit. -drive bps=xxxin bytes/s (2) only read bps limit -drive bps_rd=xxx in bytes/s (3) only write bps limit -drive bps_wr=xxx in bytes/s (4) global iops limit -drive iops=xxx in ios/s (5) only read iops limit -drive iops_rd=xxxin ios/s (6) only write iops limit -drive iops_wr=xxxin ios/s (7) the combination of some limits. -drive bps=xxx,iops=xxx Known Limitations: (1) #1 can not be used with #2, #3 together (2) #4 can not be used with #5, #6 together Yes, you might want to look into the code about domain_conf.c and qemu_driver.c part, I have already do the limitations on both XML and the command. daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py |2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c| 101 src/conf/domain_conf.h| 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms
Re: [libvirt] [RFC PATCH 0/8 v2] Summary on block IO throttle
On 10/27/2011 11:45 PM, Eric Blake wrote: On 10/27/2011 03:12 AM, Lei Li wrote: 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/kvm-one.img'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ iotune bps='n'.../ /disk 2) Enable blkio throttling setting at guest running time. virsh blkiothrottledomain device [--bpsnumber] [--bps_rdnumber] \ [--bps_wrnumber] [--iopsnumber] [--iops_rdnumber] [--iops_wrnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottledomain device Given that the XML is named iotune under disk, we should probably name the virsh command 'blkiotune' or 'disk-iotune', not 'blkiothrottle'. Hi Eric, we usediothrottle first, I changed it since Daniel P. Berrange proposediotune for per-disk element instead ofiothrottle when we discussed at RFC V1. The command 'blkiotune' already exist, supported the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices, global to the domain, but blkio throttling is specified per-disk and can vary across multiple disks. They are different two mechanism. So how about useiothrottle again? :) Also, iotune bps='n'.../ doesn't look right. This should be similar to the top-level XML. Here, taking into account the other proposal for per-block-device values (which can't be tied to individual disk): domain... blkiotune weight800/weight device path/path/to/device/path weight200/weight /device /blkiotune devices disk ... iotune bps800/bps ... /iotune /disk /devices /domain OK. I will do it at v3. daemon/remote.c | 85 + include/libvirt/libvirt.h.in| 25 + Missing changes in docs/formatdomain.html.in (to describe the new XML), docs/schemas/domaincommon.rng (to validate the new xml), and tests/ (probably qemuxml2argvdata), to test it. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] Enable the blkiothrottle command in virsh
On 10/28/2011 06:02 AM, Adam Litke wrote: On Thu, Oct 27, 2011 at 05:20:09PM +0800, Lei HH Li wrote: Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- tools/virsh.c | 99 +++ tools/virsh.pod | 13 +++ 2 files changed, 112 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 72344f0..de86c40 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6023,6 +6023,104 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * blkiothrottle command + */ +static const vshCmdInfo info_blkiothrottle[] = { +{help, N_(Set or display a block disk I/O throttle setting.)}, +{desc, N_(Set or display a block disk I/O throttle setting.)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkiothrottle[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{bps, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limits in bytes/s)}, +{bps_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limits in bytes/s)}, +{bps_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limits in bytes/s)}, +{iops, VSH_OT_INT, VSH_OFLAG_NONE, N_(total operation limits in numbers/s)}, +{iops_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read operation limits in numbers/s)}, +{iops_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write operation limits in numbers/s)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkIoThrottle(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +virDomainBlockIoThrottleInfo info; +virDomainBlockIoThrottleInfo reply; +unsigned int flags = 0; +int ret = -1; + +memset(info, 0, sizeof(info)); + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd,name))) +goto out; + +if (vshCommandOptString(cmd, device,disk) 0) +goto out; + +if (vshCommandOptULongLong(cmd, bps,info.bps) 0) { +info.bps = 0; +} + +if (vshCommandOptULongLong(cmd, bps_rd,info.bps_rd) 0) { +info.bps_rd = 0; +} + +if (vshCommandOptULongLong(cmd, bps_wr,info.bps_wr) 0) { +info.bps_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops,info.iops) 0) { +info.iops = 0; +} + +if (vshCommandOptULongLong(cmd, iops_rd,info.iops_rd) 0) { +info.iops_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops_wr,info.iops_wr) 0) { +info.bps_wr = 0; +} + +if ((info.bps == 0) (info.bps_rd == 0) (info.bps_wr == 0) + (info.iops == 0) (info.iops_rd == 0) (info.iops_wr == 0)) { What if I want to set one of these values to zero (ie. erase a current limit)? Won't this mistakenly just print out the current settings? I think you'll need a bit more sophistication here. Yes, it need to be improved, especially when support --current --live --config. I didn't write it in the summary since I think it be contained in to do list #5).. + +ret = virDomainGetBlockIoThrottle(dom, disk,reply, flags); + +if (ret != 0) +goto out; + +vshPrint(ctl, %-15s %llu\n, _(bps:), reply.bps); +vshPrint(ctl, %-15s %llu\n, _(bps_rd:), reply.bps_rd); +vshPrint(ctl, %-15s %llu\n, _(bps_wr:), reply.bps_wr); +vshPrint(ctl, %-15s %llu\n, _(iops:), reply.iops); +vshPrint(ctl, %-15s %llu\n, _(iops_rd:), reply.iops_rd); +vshPrint(ctl, %-15s %llu\n, _(iops_wr:), reply.iops_wr); + +virDomainFree(dom); +return true; +} else { +flags = 1; + +ret = virDomainSetBlockIoThrottle(dom, disk,info, flags); + +if (ret == 0) { +virDomainFree(dom); +return true; +} +} + +out: +virDomainFree(dom); +return false; +} /* * net-autostart command @@ -14017,6 +14115,7 @@ static const vshCmdDef domManagementCmds[] = { {blkiotune, cmdBlkiotune, opts_blkiotune, info_blkiotune, 0}, {blockpull, cmdBlockPull, opts_block_pull, info_block_pull, 0}, {blockjob, cmdBlockJob, opts_block_job, info_block_job, 0}, +{blkiothrottle, cmdBlkIoThrottle, opts_blkiothrottle, info_blkiothrottle, 0}, #ifndef WIN32 {console, cmdConsole, opts_console, info_console, 0}, #endif diff --git a/tools/virsh.pod b/tools/virsh.pod index 775d302..61ec613 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -572,6 +572,19 @@ operation can be checked with Bblockjob. Ipath specifies fully-qualified path of the disk. Ibandwidth specifies copying bandwidth limit in Mbps. +=item Bblkiothrottle Idomain Idevice [[I--bps Bbps] | [[I--bps_rd Bbps_rd] [I--bps_wr Bbps_wr]] [[I--iops Biops] | [[I--iops_rd Biops_rd] [I--iops_wr Biops_wr]] + +Set or display the block disk io limits settting. +Ipath specifies block disk name. +I--bps specifies total
Re: [libvirt] [PATCH 6/8] Implement Get Block IO Throttle for qemu driver
On 10/28/2011 05:53 AM, Adam Litke wrote: On Thu, Oct 27, 2011 at 05:20:08PM +0800, Lei HH Li wrote: +static int +qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, + const char *device, + virDomainBlockIoThrottleInfoPtr reply) +{ +virJSONValuePtr io_throttle; +int ret = -1; +int i; +int found = 0; + +io_throttle = virJSONValueObjectGet(result, return); + +if (!io_throttle ||io_throttle-type != VIR_JSON_TYPE_ARRAY) { ^ need a space between || and io_throttle +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_( block_io_throttle reply was missing device list )); +goto cleanup; +} + +for (i = 0; i virJSONValueArraySize(io_throttle); i++) { +virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i); +virJSONValuePtr inserted; +const char *current_dev; + + if (!temp_dev || temp_dev-type !=VIR_JSON_TYPE_OBJECT) { ^ watch spaces +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(block io throttle device entry was not in expected format)); +goto cleanup; +} + +if ((current_dev = virJSONValueObjectGetString(temp_dev, device)) == NULL) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(block io throttle device entry was not in expected format)); +goto cleanup; +} + +if(STRPREFIX(current_dev, QEMU_DRIVE_HOST_PREFIX)) +current_dev += strlen(QEMU_DRIVE_HOST_PREFIX); Is the drive prefix always going to be there? If so, I would report an error if it is missing. As written, we'll tolerate either if it's there or not. New QEMU has separate names for host guest side of the disk and libvirt gives the host side a 'drive-' prefix. The passed in dev_name is the guest side though. It's for read mode, finding the asked block device by name then get the return value of 'query-block', json-object: 'inserted'. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Add new API virDomainSetBlockIoThrottle
On 10/28/2011 04:39 AM, Adam Litke wrote: On Thu, Oct 27, 2011 at 05:20:03PM +0800, Lei HH Li wrote: This patch add new pulic API virDomainSetBlockIoThrottle to src/libvirt.c enable iotune setting for a device in domain XML. You should include the API definition for both virDomainSetBlockIoThrottle and virDomainGetBlockIoThrottle in the same patch. That way, reviewers can see the complete picture of all API changes in one place. I split it since the feature 'block I/O setting was already reviewed at RFC V1', and 'set' was implemented by supporting qmp command 'block_set_io_throttle'. 'get' was implemented by supporting qmp command 'query-block'. I thought It will be much clearer to review patches that have been split. OK, I will merge the Get method into this. Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 17 + src/conf/domain_conf.c | 76 ++ src/conf/domain_conf.h | 11 ++ src/driver.h | 11 ++ src/libvirt.c| 62 ++ src/libvirt_public.syms |1 + src/util/xml.h |2 + 7 files changed, 180 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7102bce..ff2926e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1575,6 +1575,23 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +typedef struct _virDomainBlockIoThrottleInfo virDomainBlockIoThrottleInfo; +struct _virDomainBlockIoThrottleInfo { +unsigned long long bps; +unsigned long long bps_rd; +unsigned long long bps_wr; +unsigned long long iops; +unsigned long long iops_rd; +unsigned long long iops_wr; In the last series, it was suggested that you change the names of these tuning parameters to make them more self-explanatory. I suggested: bps = total_bytes_sec : total throughput limit in bytes per second bps_rd = read_bytes_sec : read throughput limit in bytes per second bps_wr = write_bytes_sec : read throughput limit in bytes per second iops= total_iops_sec : total I/O operations per second iops_rd = read_iops_sec : read I/O operations per second iops_wr = write_iops_sec : write I/O operations per second This applies to the variable names in the above structure and to the attributes in the XML schema. +}; +typedef virDomainBlockIoThrottleInfo *virDomainBlockIoThrottleInfoPtr; + +int +virDomainSetBlockIoThrottle(virDomainPtr dom, +const char *disk, +virDomainBlockIoThrottleInfoPtr info, +unsigned int flags); + /* * NUMA support diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f9c542..8f1c65f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2444,6 +2444,41 @@ virDomainDiskDefParseXML(virCapsPtr caps, iotag = virXMLPropString(cur, io); ioeventfd = virXMLPropString(cur, ioeventfd); event_idx = virXMLPropString(cur, event_idx); +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +char *io_throttle = NULL; +io_throttle = virXMLPropString(cur, bps); +if (io_throttle) { +def-blkiothrottle.bps = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_rd); +if (io_throttle) { +def-blkiothrottle.bps_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_wr); +if (io_throttle) { +def-blkiothrottle.bps_wr = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} +io_throttle = virXMLPropString(cur, iops); +if (io_throttle) { +def-blkiothrottle.iops = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_rd); +if (io_throttle) { +def-blkiothrottle.iops_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_wr); +if (io_throttle) { +def-blkiothrottle.iops_wr = strtoull(io_throttle, NULL, 10); +} With so many attributes (bps, bps_rd, etc), I wonder if this schema should be
[libvirt] [RFC PATCH 0/8 v2] Summary on block IO throttle
Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomainBlockIoThrottle(), new command 'blkiothrottle' and Python bindings. Notes: we are sending this series out now(even though they are not completed yet.)because we want to start the review process. #1)#2) features were implemented by Zhi Yong Wu: 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/kvm-one.img'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ iotune bps='n'.../ /disk 2) Enable blkio throttling setting at guest running time. virsh blkiothrottle domain device [--bpsnumber] [--bps_rdnumber] \ [--bps_wrnumber] [--iopsnumber] [--iops_rdnumber] [--iops_wrnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device bps: bps_rd: bps_wr: iops: iops_rd: iops_wr: And I will address feedback and work on the missing features in few days includes: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. daemon/remote.c | 85 + include/libvirt/libvirt.h.in| 25 + python/generator.py |2 + python/libvirt-override-api.xml | 16 python/libvirt-override.c | 43 + src/conf/domain_conf.c | 77 src/conf/domain_conf.h | 11 +++ src/driver.h| 18 src/libvirt.c | 120 src/libvirt_public.syms |2 + src/qemu/qemu_command.c | 35 +++ src/qemu/qemu_driver.c | 108 ++ src/qemu/qemu_monitor.c | 36 src/qemu/qemu_monitor.h | 10 ++ src/qemu/qemu_monitor_json.c| 191 +++ src/qemu/qemu_monitor_json.h| 10 ++ src/qemu/qemu_monitor_text.c| 88 ++ src/qemu/qemu_monitor_text.h| 10 ++ src/remote/remote_driver.c | 81 + src/remote/remote_protocol.x| 39 - src/remote_protocol-structs | 34 +++ src/util/xml.h |3 + tools/virsh.c | 100 tools/virsh.pod | 13 +++ 24 files changed, 1156 insertions(+), 1 deletions(-) -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] Add new API virDomainSetBlockIoThrottle
This patch add new pulic API virDomainSetBlockIoThrottle to src/libvirt.c enable iotune setting for a device in domain XML. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 17 + src/conf/domain_conf.c | 76 ++ src/conf/domain_conf.h | 11 ++ src/driver.h | 11 ++ src/libvirt.c| 62 ++ src/libvirt_public.syms |1 + src/util/xml.h |2 + 7 files changed, 180 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7102bce..ff2926e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1575,6 +1575,23 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +typedef struct _virDomainBlockIoThrottleInfo virDomainBlockIoThrottleInfo; +struct _virDomainBlockIoThrottleInfo { +unsigned long long bps; +unsigned long long bps_rd; +unsigned long long bps_wr; +unsigned long long iops; +unsigned long long iops_rd; +unsigned long long iops_wr; +}; +typedef virDomainBlockIoThrottleInfo *virDomainBlockIoThrottleInfoPtr; + +int +virDomainSetBlockIoThrottle(virDomainPtr dom, +const char *disk, +virDomainBlockIoThrottleInfoPtr info, +unsigned int flags); + /* * NUMA support diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f9c542..8f1c65f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2444,6 +2444,41 @@ virDomainDiskDefParseXML(virCapsPtr caps, iotag = virXMLPropString(cur, io); ioeventfd = virXMLPropString(cur, ioeventfd); event_idx = virXMLPropString(cur, event_idx); +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +char *io_throttle = NULL; +io_throttle = virXMLPropString(cur, bps); +if (io_throttle) { +def-blkiothrottle.bps = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_rd); +if (io_throttle) { +def-blkiothrottle.bps_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_wr); +if (io_throttle) { +def-blkiothrottle.bps_wr = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} +io_throttle = virXMLPropString(cur, iops); +if (io_throttle) { +def-blkiothrottle.iops = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_rd); +if (io_throttle) { +def-blkiothrottle.iops_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_wr); +if (io_throttle) { +def-blkiothrottle.iops_wr = strtoull(io_throttle, NULL, 10); +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -9317,6 +9352,47 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, target dev='%s' bus='%s'/\n, def-dst, bus); +/*disk I/O throttling*/ +if (def-blkiothrottle.bps +|| def-blkiothrottle.bps_rd +|| def-blkiothrottle.bps_wr +|| def-blkiothrottle.iops +|| def-blkiothrottle.iops_rd +|| def-blkiothrottle.iops_wr) { +virBufferAsprintf(buf, iotune); +if (def-blkiothrottle.bps) { +virBufferAsprintf(buf, bps='%llu', + def-blkiothrottle.bps); +} + +if (def-blkiothrottle.bps_rd) { +virBufferAsprintf(buf, bps_rd='%llu', + def-blkiothrottle.bps_rd); +} + +if (def-blkiothrottle.bps_wr) { +virBufferAsprintf(buf, bps_wr='%llu', + def-blkiothrottle.bps_wr); +} + +if (def-blkiothrottle.iops) { +virBufferAsprintf(buf, iops='%llu', + def-blkiothrottle.iops); +} + +if (def-blkiothrottle.iops_rd) { +virBufferAsprintf(buf, iops_rd='%llu
[libvirt] [PATCH 4/8] Add new API virDomainGetBlockIoThrottle
Support virDomainGetBlockIoThrottle to src/libvirt.c Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in |5 +++ src/driver.h |6 src/libvirt.c| 57 ++ src/libvirt_public.syms |1 + 4 files changed, 69 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ff2926e..a844f61 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1592,6 +1592,11 @@ virDomainSetBlockIoThrottle(virDomainPtr dom, virDomainBlockIoThrottleInfoPtr info, unsigned int flags); +int +virDomainGetBlockIoThrottle(virDomainPtr dom, +const char *disk, +virDomainBlockIoThrottleInfoPtr reply, +unsigned int flags); /* * NUMA support diff --git a/src/driver.h b/src/driver.h index ffa71c8..936fc09 100644 --- a/src/driver.h +++ b/src/driver.h @@ -745,6 +745,11 @@ typedef int virDomainBlockIoThrottleInfoPtr info, unsigned int flags); +typedef int +(*virDrvDomainGetBlockIoThrottle)(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags); /** * _virDriver: @@ -904,6 +909,7 @@ struct _virDriver { virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; virDrvDomainSetBlockIoThrottle domainSetBlockIoThrottle; +virDrvDomainGetBlockIoThrottle domainGetBlockIoThrottle; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index dfc74fb..cf6ce38 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17026,3 +17026,60 @@ error: return -1; } +/** + * virDomainGetBlockIoThrottle: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @reply: specify block I/O info in bytes + * @flags: indicate if it set or display block I/O limits info + * + * This function is mainly to enable Block I/O throttling function in libvirt. + * It is used to get the block I/O throttling setting for specified domain. + * + * Returns 0 if the operation has started, -1 on failure. + */ + +int virDomainGetBlockIoThrottle(virDomainPtr dom, +const char *disk, +virDomainBlockIoThrottleInfoPtr reply, +unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(dom, disk=%p, reply=%p, flags=%x, + disk, reply, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN (dom)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +conn = dom-conn; + +if (dom-conn-flags VIR_CONNECT_RO) { +virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (!disk) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +if (conn-driver-domainGetBlockIoThrottle) { +int ret; +ret = conn-driver-domainGetBlockIoThrottle(dom, disk, reply, flags); +if (ret 0) +goto error; +return ret; +} + +virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(dom-conn); +return -1; + +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ce29978..3acfa6c 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -496,6 +496,7 @@ LIBVIRT_0.9.7 { virDomainSnapshotListChildrenNames; virDomainSnapshotNumChildren; virDomainSetBlockIoThrottle; +virDomainGetBlockIoThrottle; } LIBVIRT_0.9.5; # define new API here using predicted next version number -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] Getting Block IO Throttle support to remote driver
Add getting Block IO Throttle support to remote driver. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- daemon/remote.c | 43 ++ src/remote/remote_driver.c | 42 + src/remote/remote_protocol.x | 23 ++ src/remote_protocol-structs | 21 4 files changed, 129 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index c604556..ae8d5ed 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1890,6 +1890,49 @@ cleanup: return rv; } +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +virDomainBlockIoThrottleInfo reply; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +rv = virDomainGetBlockIoThrottle(dom, args-disk, reply, args-flags); + +if (rv 0) +goto cleanup; + +ret-bps = reply.bps; +ret-bps_rd = reply.bps_rd; +ret-bps_wr = reply.bps_wr; +ret-iops= reply.iops; +ret-iops_rd = reply.iops_rd; +ret-iops_wr = reply.iops_wr; +ret-found = 1; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + /*-*/ static int diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a2d78f0..3c54184 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2187,6 +2187,47 @@ done: return rv; } +static int remoteDomainGetBlockIoThrottle(virDomainPtr domain, + const char *disk, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags) +{ +int rv = -1; +remote_domain_get_block_io_throttle_args args; +remote_domain_get_block_io_throttle_ret ret; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv); + +make_nonnull_domain(args.dom, domain); +args.disk = (char *)disk; +args.flags = flags; + +memset(ret, 0, sizeof(ret)); + +if (call(domain-conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_get_block_io_throttle_args, + (char *) args, + (xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret, + (char *) ret) == -1) { +goto done; +} + +if (ret.found) { +reply-bps = ret.bps; +reply-bps_rd = ret.bps_rd; +reply-bps_wr = ret.bps_wr; +reply-iops= ret.iops; +reply-iops_rd = ret.iops_rd; +reply-iops_wr = ret.iops_wr; +} +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + /*--*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -4512,6 +4553,7 @@ static virDriver remote_driver = { .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */ .domainSetBlockIoThrottle = remoteDomainSetBlockIoThrottle, /* 0.9.8 */ +.domainGetBlockIoThrottle = remoteDomainGetBlockIoThrottle, /* 0.9.8 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index b467952..46774cc 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1085,6 +1085,28 @@ struct remote_domain_set_block_io_throttle_args { unsigned int flags; }; +struct remote_domain_get_block_io_throttle_args { +remote_nonnull_domain dom; +remote_nonnull_string disk; +unsigned hyper bps; +unsigned hyper bps_rd; +unsigned hyper bps_wr; +unsigned hyper iops; +unsigned hyper iops_rd; +unsigned hyper iops_wr; +unsigned int flags; +}; + +struct remote_domain_get_block_io_throttle_ret { +unsigned hyper bps; +unsigned hyper bps_rd; +unsigned hyper bps_wr; +unsigned hyper iops; +unsigned hyper iops_rd; +unsigned hyper iops_wr; +unsigned int found; +}; + /* Network calls: */ struct
[libvirt] [RFC PATCH 0/8 v2] Summary on block IO throttle
Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomainBlockIoThrottle(), new command 'blkiothrottle' and Python bindings. Notes: we are sending this series out now(even though they are not completed yet.)because we want to start the review process. #1)#2) features were implemented by Zhi Yong Wu: 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/kvm-one.img'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ iotune bps='n'.../ /disk 2) Enable blkio throttling setting at guest running time. virsh blkiothrottle domain device [--bpsnumber] [--bps_rdnumber] \ [--bps_wrnumber] [--iopsnumber] [--iops_rdnumber] [--iops_wrnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device bps: bps_rd: bps_wr: iops: iops_rd: iops_wr: And I will address feedback and work on the missing features in few days includes: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. daemon/remote.c | 85 + include/libvirt/libvirt.h.in| 25 + python/generator.py |2 + python/libvirt-override-api.xml | 16 python/libvirt-override.c | 43 + src/conf/domain_conf.c | 77 src/conf/domain_conf.h | 11 +++ src/driver.h| 18 src/libvirt.c | 120 src/libvirt_public.syms |2 + src/qemu/qemu_command.c | 35 +++ src/qemu/qemu_driver.c | 108 ++ src/qemu/qemu_monitor.c | 36 src/qemu/qemu_monitor.h | 10 ++ src/qemu/qemu_monitor_json.c| 191 +++ src/qemu/qemu_monitor_json.h| 10 ++ src/qemu/qemu_monitor_text.c| 88 ++ src/qemu/qemu_monitor_text.h| 10 ++ src/remote/remote_driver.c | 81 + src/remote/remote_protocol.x| 39 - src/remote_protocol-structs | 34 +++ src/util/xml.h |3 + tools/virsh.c | 100 tools/virsh.pod | 13 +++ 24 files changed, 1156 insertions(+), 1 deletions(-) -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] Support virDomainGetBlockIoThrottle in the python API
Signed-off-by:Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- python/generator.py |2 + python/libvirt-override-api.xml | 16 ++ python/libvirt-override.c | 43 +++ 3 files changed, 61 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 71afdb7..9d786c4 100755 --- a/python/generator.py +++ b/python/generator.py @@ -414,6 +414,8 @@ skip_impl = ( 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', 'virDomainBlockStatsFlags', +'virDomainSetBlockIoThrottle', +'virDomainGetBlockIoThrottle', ) qemu_skip_impl = ( diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..5d85923 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -375,5 +375,21 @@ arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/ return type='unsigned long' info='current max migration speed, or None in case of error'/ /function +function name='virDomainSetBlockIoThrottle' file='python' + infoChange the I/O throttle for a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='info' type='virDomainBlockIoThrottleInfoPtr' info='io throttle limits'/ + arg name='flags' type='unsigned int' info='0 on display, 1 on set.'/ + return type='virDomainSetBlockIoThrottleInfo' info='0 in case of operation has started, -1 in case of failure'/ +/function +function name='virDomainGetBlockIoThrottle' file='python' + infoGet the I/O throttle for a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='reply' type='virDomainBlockIoThrottleInfoPtr' info='io throttle info'/ + arg name='flags' type='unsigned int' info='0 on display, 1 on set.'/ + return type='virDomainGetBlockIoThrottleInfo' info='A dictionary containing block I/O limits information.'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1759bae..c005cf0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3195,6 +3195,48 @@ LIBVIRT_END_ALLOW_THREADS; return ret; } +static PyObject * +libvirt_virDomainGetBlockIoThrottle(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain; +const char *disk; +unsigned int flags; +virDomainBlockIoThrottleInfo reply; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainGetBlockIoThrottle, + pyobj_domain, disk, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainGetBlockIoThrottle(domain, disk, reply, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret != 1) +return VIR_PY_NONE; + +if ((ret = PyDict_New()) == NULL) +return VIR_PY_NONE; + +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps), + libvirt_intWrap(reply.bps)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps_rd), + libvirt_intWrap(reply.bps_rd)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps_wr), + libvirt_intWrap(reply.bps_wr)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops), + libvirt_intWrap(reply.iops)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops_rd), + libvirt_intWrap(reply.iops_rd)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops_wr), + libvirt_intWrap(reply.iops_wr)); +return ret; +} + /*** * Helper functions to avoid importing modules * for every callback @@ -4837,6 +4879,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) virDomainSnapshotListNames, libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, {(char *) virDomainRevertToSnapshot, libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, {(char *) virDomainGetBlockJobInfo, libvirt_virDomainGetBlockJobInfo, METH_VARARGS, NULL}, +{(char *) virDomainGetBlockIoThrottle, libvirt_virDomainGetBlockIoThrottle, METH_VARARGS, NULL}, {(char *) virDomainSendKey, libvirt_virDomainSendKey, METH_VARARGS, NULL}, {(char *) virDomainMigrateGetMaxSpeed, libvirt_virDomainMigrateGetMaxSpeed, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] Implement Block Io Throttle setting for the qemu driver
This patch implement the block io throtte setting for the qemu driver through - qmp/hmp command 'block_set_io_throttle'. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/qemu/qemu_command.c | 35 +++ src/qemu/qemu_driver.c | 53 ++ src/qemu/qemu_monitor.c | 18 ++ src/qemu/qemu_monitor.h |5 src/qemu/qemu_monitor_json.c | 45 +++ src/qemu/qemu_monitor_json.h |5 src/qemu/qemu_monitor_text.c | 39 ++ src/qemu/qemu_monitor_text.h |5 8 files changed, 205 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0c5bfab..9422e17 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1734,6 +1734,41 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } } +/*block I/O throttling*/ +if (disk-blkiothrottle.bps || disk-blkiothrottle.bps_rd +|| disk-blkiothrottle.bps_wr || disk-blkiothrottle.iops +|| disk-blkiothrottle.iops_rd || disk-blkiothrottle.iops_wr) { +if (disk-blkiothrottle.bps) { +virBufferAsprintf(opt, ,bps=%llu, + disk-blkiothrottle.bps); +} + +if (disk-blkiothrottle.bps_rd) { +virBufferAsprintf(opt, ,bps_rd=%llu, + disk-blkiothrottle.bps_rd); +} + +if (disk-blkiothrottle.bps_wr) { +virBufferAsprintf(opt, ,bps_wr=%llu, + disk-blkiothrottle.bps_wr); +} + +if (disk-blkiothrottle.iops) { +virBufferAsprintf(opt, ,iops=%llu, + disk-blkiothrottle.iops); +} + +if (disk-blkiothrottle.iops_rd) { +virBufferAsprintf(opt, ,iops_rd=%llu, + disk-blkiothrottle.iops_rd); +} + +if (disk-blkiothrottle.iops_wr) { +virBufferAsprintf(opt, ,iops_wr=%llu, + disk-blkiothrottle.iops_wr); +} +} + if (virBufferError(opt)) { virReportOOMError(); goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e053a97..b41a7d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10660,6 +10660,58 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, return ret; } +static int +qemuDomainSetBlockIoThrottle(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +%s, _(domain is not running)); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +priv = vm-privateData; +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, flags); +qemuDomainObjExitMonitorWithDriver(driver, vm); +if (qemuDomainObjEndJob(driver, vm) == 0) { +vm = NULL; +goto cleanup; +} + +cleanup: +VIR_FREE(device); +if (vm) +virDomainObjUnlock(vm); +qemuDriverUnlock(driver); +return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU, @@ -10802,6 +10854,7 @@ static virDriver qemuDriver = { .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */ +.domainSetBlockIoThrottle = qemuDomainSetBlockIoThrottle, /* 0.9.8 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 182e63d..b061631 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2542,6 +2542,24 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, return ret; } +int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoThrottleInfoPtr info, + unsigned int
[libvirt] [PATCH 7/8] Enable the blkiothrottle command in virsh
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- tools/virsh.c | 99 +++ tools/virsh.pod | 13 +++ 2 files changed, 112 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 72344f0..de86c40 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6023,6 +6023,104 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * blkiothrottle command + */ +static const vshCmdInfo info_blkiothrottle[] = { +{help, N_(Set or display a block disk I/O throttle setting.)}, +{desc, N_(Set or display a block disk I/O throttle setting.)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkiothrottle[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{bps, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limits in bytes/s)}, +{bps_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limits in bytes/s)}, +{bps_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limits in bytes/s)}, +{iops, VSH_OT_INT, VSH_OFLAG_NONE, N_(total operation limits in numbers/s)}, +{iops_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read operation limits in numbers/s)}, +{iops_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write operation limits in numbers/s)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkIoThrottle(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +virDomainBlockIoThrottleInfo info; +virDomainBlockIoThrottleInfo reply; +unsigned int flags = 0; +int ret = -1; + +memset(info, 0, sizeof(info)); + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if (vshCommandOptULongLong(cmd, bps, info.bps) 0) { +info.bps = 0; +} + +if (vshCommandOptULongLong(cmd, bps_rd, info.bps_rd) 0) { +info.bps_rd = 0; +} + +if (vshCommandOptULongLong(cmd, bps_wr, info.bps_wr) 0) { +info.bps_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops, info.iops) 0) { +info.iops = 0; +} + +if (vshCommandOptULongLong(cmd, iops_rd, info.iops_rd) 0) { +info.iops_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops_wr, info.iops_wr) 0) { +info.bps_wr = 0; +} + +if ((info.bps == 0) (info.bps_rd == 0) (info.bps_wr == 0) + (info.iops == 0) (info.iops_rd == 0) (info.iops_wr == 0)) { + +ret = virDomainGetBlockIoThrottle(dom, disk, reply, flags); + +if (ret != 0) +goto out; + +vshPrint(ctl, %-15s %llu\n, _(bps:), reply.bps); +vshPrint(ctl, %-15s %llu\n, _(bps_rd:), reply.bps_rd); +vshPrint(ctl, %-15s %llu\n, _(bps_wr:), reply.bps_wr); +vshPrint(ctl, %-15s %llu\n, _(iops:), reply.iops); +vshPrint(ctl, %-15s %llu\n, _(iops_rd:), reply.iops_rd); +vshPrint(ctl, %-15s %llu\n, _(iops_wr:), reply.iops_wr); + +virDomainFree(dom); +return true; +} else { +flags = 1; + +ret = virDomainSetBlockIoThrottle(dom, disk, info, flags); + +if (ret == 0) { +virDomainFree(dom); +return true; +} +} + +out: +virDomainFree(dom); +return false; +} /* * net-autostart command @@ -14017,6 +14115,7 @@ static const vshCmdDef domManagementCmds[] = { {blkiotune, cmdBlkiotune, opts_blkiotune, info_blkiotune, 0}, {blockpull, cmdBlockPull, opts_block_pull, info_block_pull, 0}, {blockjob, cmdBlockJob, opts_block_job, info_block_job, 0}, +{blkiothrottle, cmdBlkIoThrottle, opts_blkiothrottle, info_blkiothrottle, 0}, #ifndef WIN32 {console, cmdConsole, opts_console, info_console, 0}, #endif diff --git a/tools/virsh.pod b/tools/virsh.pod index 775d302..61ec613 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -572,6 +572,19 @@ operation can be checked with Bblockjob. Ipath specifies fully-qualified path of the disk. Ibandwidth specifies copying bandwidth limit in Mbps. +=item Bblkiothrottle Idomain Idevice [[I--bps Bbps] | [[I--bps_rd Bbps_rd] [I--bps_wr Bbps_wr]] [[I--iops Biops] | [[I--iops_rd Biops_rd] [I--iops_wr Biops_wr]] + +Set or display the block disk io limits settting. +Ipath specifies block disk name. +I--bps specifies total throughput limit in bytes/s. +I--bps_rd specifies read throughput limit in bytes/s. +I--bps_wr specifies write throughput limit in bytes/s. +I--iops specifies total operation limit in numbers/s. +I--iops_rd specifies read operation limit in numbers/s. +I--iops_wr specifies write operation limit in numbers/s. + +If no limit is specified, it will query current I/O limits setting. + =item Bblockjob Idomain Ipath [I--abort] [I--info] [Ibandwidth] Manage active
Re: [libvirt] [PATCH v4 2/2] add interface for blkio.weight_device
On 10/12/2011 05:26 PM, Hu Tao wrote: This patch adds a parameter --weight-device to virsh command blkiotune for setting/getting blkio.weight_device. --- daemon/remote.c |2 +- include/libvirt/libvirt.h.in |9 ++ src/conf/domain_conf.c | 129 ++- src/conf/domain_conf.h | 16 src/libvirt_private.syms |2 + src/qemu/qemu_cgroup.c | 22 + src/qemu/qemu_driver.c | 199 +++-- src/util/cgroup.c| 33 +++ src/util/cgroup.h|3 + tools/virsh.c| 69 +-- tools/virsh.pod |5 +- 11 files changed, 467 insertions(+), 22 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 520fef2..7691b08 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1619,7 +1619,7 @@ success: cleanup: if (rv 0) virNetMessageSaveError(rerr); -VIR_FREE(params); +virTypedParameterFree(params, nparams); if (dom) virDomainFree(dom); return rv; diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index fa53147..84178c2 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1162,6 +1162,15 @@ char * virDomainGetSchedulerType(virDomainPtr domain, #define VIR_DOMAIN_BLKIO_WEIGHT weight +/** + * VIR_DOMAIN_BLKIO_WEIGHT_DEVICE: + * + * Macro for the blkio tunable weight_device: it represents the + * per-device weight. This name refers to a VIR_TYPED_PARAM_STRING. + */ + +#define VIR_DOMAIN_BLKIO_WEIGHT_DEVICE weight_device + /* Set Blkio tunables for the domain*/ int virDomainSetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b9ddf26..96723ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -580,6 +580,97 @@ VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST, #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE + +void virBlkioWeightDeviceFree(virBlkioWeightDevicePtr deviceWeights, + int ndevices) +{ +int i; + +for (i = 0; i ndevices; i++) +VIR_FREE(deviceWeights[i].path); +VIR_FREE(deviceWeights); +} + +/** + * virBlkioWeightDeviceToStr: + * + * This function returns a string representing device weights that is + * suitable for writing to /cgroup/blkio/blkio.weight_device, given + * a list of weight devices. + */ +#if defined(major) defined(minor) +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr weightdevices, + int ndevices, + char **result) +{ +int i; +struct stat s; +virBuffer buf = VIR_BUFFER_INITIALIZER; + +for (i = 0; i ndevices; i++) { +if (stat(weightdevices[i].path,s) == -1) +return -1; +if ((s.st_mode S_IFMT) != S_IFBLK) +return -1; +virBufferAsprintf(buf, %d:%d %d\n, + major(s.st_rdev), + minor(s.st_rdev), + weightdevices[i].weight); +} + +*result = virBufferContentAndReset(buf); +return 0; +} +#else +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr weightdevices ATTRIBUTE_UNUSED, + int ndevices ATTRIBUTE_UNUSED, + char **result ATTRIBUTE_UNUSED) +{ +return -1; +} +#endif + +/** + * virDomainBlkioWeightDeviceParseXML + * + * this function parses a XML node: + * + *device + *path/fully/qualified/device/path/path + *weightweight/weight + */device + * + * and fills a virBlkioWeightDevice struct. + */ +static int virDomainBlkioWeightDeviceParseXML(xmlNodePtr root, + virBlkioWeightDevicePtr dw) +{ +char *c; +xmlNodePtr node; + +if (!dw) +return -1; + +node = root-children; +while (node) { +if (node-type == XML_ELEMENT_NODE) { +if (xmlStrEqual(node-name, BAD_CAST path)) { +dw-path = (char *)xmlNodeGetContent(node); +} else if (xmlStrEqual(node-name, BAD_CAST weight)) { +c = (char *)xmlNodeGetContent(node); +if (virStrToLong_i(c, NULL, 10,dw-weight) 0) +return -1; +VIR_FREE(c); +} +} +node = node-next; +} + +return 0; +} + + + static void virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { @@ -1244,6 +1335,8 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def-emulator); VIR_FREE(def-description); +virBlkioWeightDeviceFree(def-blkio.weight_devices, def-blkio.ndevices); + virDomainWatchdogDefFree(def-watchdog);
[libvirt] [RFC PATCH 0/5] Summary on QEMU I/O throttling support to libvirt
Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomainBlockIoThrottle(), new command 'blkiothrottle' and Python bindings. Notes: we are sending this series out now(even though they are not completed yet.)because we want to start the review process. These patches implemented the below features by Zhi Yong Wu: 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/kvm-one.img'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ iothrottle bps='10' bps_rd='2'.../ /disk 2) Enable blkio throttling setting at guest running time. virsh blkiothrottledomain device [--bpsnumber] [--bps_rdnumber] \ [--bps_wrnumber] [--iopsnumber] [--iops_rdnumber] [--iops_wrnumber] [PATCH 1/5] Add new API virDomainBlockIoThrottle. [PATCH 2/5] Add virDomainBlockIoThrottle support to the remote driver. [PATCH 3/5] Implement virDomainBlockIoThrottle for the qemu driver. [PATCH 4/5] Enable the virDomainBlockIoThrottle API in virsh. [PATCH 5/5] Enable virDomainBlockIoThrottle in the python API. And I will address feedback and work on the missing features in few days includes: 3) The support to display the setting for block i/o throttling - HMP/QMP 4) Python binding support. 5) --current --live --config options support to unify the libvirt API. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 1/5] Add new API virDomainBlockIoThrottle
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 22 src/conf/domain_conf.c | 77 ++ src/conf/domain_conf.h | 11 ++ src/driver.h | 11 ++ src/libvirt.c| 66 src/libvirt_public.syms |1 + src/util/xml.h |3 ++ 7 files changed, 191 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 07617be..f7b892d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1573,6 +1573,28 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef unsigned long long virDomainBlockIoThrottleUnits; + +typedef struct _virDomainBlockIoThrottleInfo virDomainBlockIoThrottleInfo; +struct _virDomainBlockIoThrottleInfo { +virDomainBlockIoThrottleUnits bps; +virDomainBlockIoThrottleUnits bps_rd; +virDomainBlockIoThrottleUnits bps_wr; +virDomainBlockIoThrottleUnits iops; +virDomainBlockIoThrottleUnits iops_rd; +virDomainBlockIoThrottleUnits iops_wr; +}; +typedef virDomainBlockIoThrottleInfo *virDomainBlockIoThrottleInfoPtr; + +intvirDomainBlockIoThrottle(virDomainPtr dom, +const char *disk, +virDomainBlockIoThrottleInfoPtr info, +virDomainBlockIoThrottleInfoPtr reply, +unsigned int flags); /* * NUMA support diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 944cfa9..d0ba07e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2422,6 +2422,42 @@ virDomainDiskDefParseXML(virCapsPtr caps, iotag = virXMLPropString(cur, io); ioeventfd = virXMLPropString(cur, ioeventfd); event_idx = virXMLPropString(cur, event_idx); +} else if (xmlStrEqual(cur-name, BAD_CAST iothrottle)) { +char *io_throttle = NULL; +io_throttle = virXMLPropString(cur, bps); +if (io_throttle) { +def-blkiothrottle.bps = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_rd); +if (io_throttle) { +def-blkiothrottle.bps_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, bps_wr); +if (io_throttle) { +def-blkiothrottle.bps_wr = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops); +if (io_throttle) { +def-blkiothrottle.iops = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_rd); +if (io_throttle) { +def-blkiothrottle.iops_rd = strtoull(io_throttle, NULL, 10); +VIR_FREE(io_throttle); +} + +io_throttle = virXMLPropString(cur, iops_wr); +if (io_throttle) { +def-blkiothrottle.iops_wr = strtoull(io_throttle, NULL, 10); +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -9249,6 +9285,47 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, target dev='%s' bus='%s'/\n, def-dst, bus); +/*disk I/O throttling*/ +if (def-blkiothrottle.bps +|| def-blkiothrottle.bps_rd +|| def-blkiothrottle.bps_wr +|| def-blkiothrottle.iops +|| def-blkiothrottle.iops_rd +|| def-blkiothrottle.iops_wr) { +virBufferAsprintf(buf, iothrottle); +if (def-blkiothrottle.bps) { +virBufferAsprintf(buf, bps='%llu', + def-blkiothrottle.bps); +} + +if (def-blkiothrottle.bps_rd) { +virBufferAsprintf(buf, bps_rd='%llu', + def-blkiothrottle.bps_rd); +} + +if (def-blkiothrottle.bps_wr) { +virBufferAsprintf(buf, bps_wr='%llu', + def-blkiothrottle.bps_wr); +} + +if (def-blkiothrottle.iops) { +virBufferAsprintf(buf, iops='%llu', + def-blkiothrottle.iops); +} + +if
[libvirt] [RFC PATCH 2/5] Add virDomainBlockIoThrottle support to the remote driver
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- daemon/remote.c | 51 src/remote/remote_driver.c | 53 ++ src/remote/remote_protocol.x | 24 ++- src/remote_protocol-structs | 21 +++- 4 files changed, 147 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 245d41c..2b277b3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1796,6 +1796,57 @@ cleanup: return rv; } +static int +remoteDispatchDomainBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, +virNetServerClientPtr client ATTRIBUTE_UNUSED, +virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, +virNetMessageErrorPtr rerr, +remote_domain_block_io_throttle_args *args, +remote_domain_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +virDomainBlockIoThrottleInfo tmp; +virDomainBlockIoThrottleInfo reply; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (args) { +tmp.bps = args-bps; +tmp.bps_rd = args-bps_rd; +tmp.bps_wr = args-bps_wr; +tmp.iops= args-iops; +tmp.iops_rd = args-iops_rd; +tmp.iops_wr = args-iops_wr; +} + +rv = virDomainBlockIoThrottle(dom, args-disk, tmp, reply, args-flags); +if (rv = 0) +goto cleanup; + +ret-bps = reply.bps; +ret-bps_rd = reply.bps_rd; +ret-bps_wr = reply.bps_wr; +ret-iops= reply.iops; +ret-iops_rd = reply.iops_rd; +ret-iops_wr = reply.iops_wr; +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} /*-*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 2b2f41e..5dbc793 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2136,6 +2136,58 @@ done: return rv; } +static int remoteDomainBlockIoThrottle(virDomainPtr domain, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags) +{ +int rv = -1; +remote_domain_block_io_throttle_args args; +remote_domain_block_io_throttle_ret ret; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv); + +memset(args, 0, sizeof(args)); +memset(ret, 0, sizeof(ret)); + +make_nonnull_domain(args.dom, domain); +args.disk = (char *)disk; +args.bps = info-bps; +args.bps_rd = info-bps_rd; +args.bps_wr = info-bps_wr; +args.iops = info-iops; +args.iops_rd = info-iops_rd; +args.iops_wr = info-iops_wr; +args.flags = flags; + +if (call(domain-conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_block_io_throttle_args, + (char *) args, + (xdrproc_t) xdr_remote_domain_block_io_throttle_ret, + (char *) ret) == -1) { +goto done; +} + +if (ret.bps || ret.bps_rd || ret.bps_wr +|| ret.iops || ret.iops_rd || ret.iops_wr) { +reply-bps = ret.bps; +reply-bps_rd = ret.bps_rd; +reply-bps_wr = ret.bps_wr; +reply-iops= ret.iops; +reply-iops_rd = ret.iops_rd; +reply-iops_wr = ret.iops_wr; +rv = 1; +} else { +rv = 0; +} + +done: +remoteDriverUnlock(priv); +return rv; +} + /*--*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -4431,6 +4483,7 @@ static virDriver remote_driver = { .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */ +.domainBlockIoThrottle = remoteDomainBlockIoThrottle, /* 0.9.4 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c8a92fd..ab11a70 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1073,6 +1073,27 @@ struct remote_domain_block_pull_args { unsigned int flags; }; +struct remote_domain_block_io_throttle_args { +remote_nonnull_domain dom; +remote_nonnull_string disk; +unsigned
[libvirt] [RFC PATCH 4/5] Enable the virDomainBlockIoThrottle API in virsh
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- tools/virsh.c | 86 +++ tools/virsh.pod | 13 2 files changed, 99 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 9532bc3..51487b7 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6019,6 +6019,91 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * blkiothrottle command + */ +static const vshCmdInfo info_blkiothrottle[] = { +{help, N_(Set or display a block disk I/O throttle setting.)}, +{desc, N_(Set or display a block disk I/O throttle setting.)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkiothrottle[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{bps, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limits in bytes/s)}, +{bps_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limits in bytes/s)}, +{bps_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limits in bytes/s)}, +{iops, VSH_OT_INT, VSH_OFLAG_NONE, N_(total operation limits in numbers/s)}, +{iops_rd, VSH_OT_INT, VSH_OFLAG_NONE, N_(read operation limits in numbers/s)}, +{iops_wr, VSH_OT_INT, VSH_OFLAG_NONE, N_(write operation limits in numbers/s)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkIoThrottle(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +virDomainBlockIoThrottleInfo info; +virDomainBlockIoThrottleInfo reply; +unsigned int flags = 0; +int ret = -1; + +memset(info, 0, sizeof(info)); + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if (vshCommandOptULongLong(cmd, bps, info.bps) 0) { +info.bps = 0; +} + +if (vshCommandOptULongLong(cmd, bps_rd, info.bps_rd) 0) { +info.bps_rd = 0; +} + +if (vshCommandOptULongLong(cmd, bps_wr, info.bps_wr) 0) { +info.bps_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops, info.iops) 0) { +info.iops = 0; +} + +if (vshCommandOptULongLong(cmd, iops_rd, info.iops_rd) 0) { +info.iops_wr = 0; +} + +if (vshCommandOptULongLong(cmd, iops_wr, info.iops_wr) 0) { +info.bps_wr = 0; +} + +if ((info.bps == 0) (info.bps_rd == 0) (info.bps_wr == 0) + (info.iops == 0) (info.iops_rd == 0) (info.iops_wr == 0)) { +flags = 0; +} else { +flags = 1; +} + +ret = virDomainBlockIoThrottle(dom, disk, info, reply, flags); + +if (ret == 0) { +virDomainFree(dom); +return true; +} + +out: +virDomainFree(dom); +return false; +} + /* * net-autostart command @@ -13712,6 +13797,7 @@ static const vshCmdDef domManagementCmds[] = { {blkiotune, cmdBlkiotune, opts_blkiotune, info_blkiotune, 0}, {blockpull, cmdBlockPull, opts_block_pull, info_block_pull, 0}, {blockjob, cmdBlockJob, opts_block_job, info_block_job, 0}, +{blkiothrottle, cmdBlkIoThrottle, opts_blkiothrottle, info_blkiothrottle, 0}, #ifndef WIN32 {console, cmdConsole, opts_console, info_console, 0}, #endif diff --git a/tools/virsh.pod b/tools/virsh.pod index 1f7c76f..5b52980 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -572,6 +572,19 @@ operation can be checked with Bblockjob. Ipath specifies fully-qualified path of the disk. Ibandwidth specifies copying bandwidth limit in Mbps. +=item Bblkiothrottle Idomain Idevice [[I--bps Bbps] | [[I--bps_rd Bbps_rd] [I--bps_wr Bbps_wr]] [[I--iops Biops] | [[I--iops_rd Biops_rd] [I--iops_wr Biops_wr]] + +Set or display the block disk io limits settting. +Ipath specifies block disk name. +I--bps specifies total throughput limit in bytes/s. +I--bps_rd specifies read throughput limit in bytes/s. +I--bps_wr specifies write throughput limit in bytes/s. +I--iops specifies total operation limit in numbers/s. +I--iops_rd specifies read operation limit in numbers/s. +I--iops_wr specifies write operation limit in numbers/s. + +If no limit is specified, it will query current I/O limits setting. + =item Bblockjob Idomain Ipath [I--abort] [I--info] [Ibandwidth] Manage active block operations. -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 5/5] Enable virDomainBlockIoThrottle in the python API
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- python/generator.py |1 + python/libvirt-override-api.xml |9 python/libvirt-override.c | 43 +++ 3 files changed, 53 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 79558dd..bec9e83 100755 --- a/python/generator.py +++ b/python/generator.py @@ -413,6 +413,7 @@ skip_impl = ( 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', 'virDomainBlockStatsFlags', +'virDomainBlockIoThrottle', ) qemu_skip_impl = ( diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 3013e46..3044b33 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -369,5 +369,14 @@ arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/ return type='unsigned long' info='current max migration speed, or None in case of error'/ /function +function name='virDomainBlockIoThrottle' file='python' + infoI/O throttle a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='info' type='virDomainBlockIoThrottleInfoPtr' info='io throttle limits'/ + arg name='reply' type='virDomainBlockIoThrottleInfoPtr' info='io throttle info'/ + arg name='flags' type='unsigned int' info='1 on display, 1 on set.'/ + return type='virDomainBlockIoThrottleInfo' info='A dictionary containing block I/O limits information.'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d65423d..767906a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3150,6 +3150,48 @@ LIBVIRT_END_ALLOW_THREADS; return ret; } +static PyObject * +libvirt_virDomainBlockIoThrottle(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain; +const char *disk; +unsigned int flags; +virDomainBlockIoThrottleInfo reply; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainBlockIoThrottle, + pyobj_domain, disk, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainBlockIoThrottle(domain, disk, NULL, reply, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret != 1) +return VIR_PY_NONE; + +if ((ret = PyDict_New()) == NULL) +return VIR_PY_NONE; + +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps), + libvirt_intWrap(reply.bps)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps_rd), + libvirt_intWrap(reply.bps_rd)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(bps_wr), + libvirt_intWrap(reply.bps_wr)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops), + libvirt_intWrap(reply.iops)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops_rd), + libvirt_intWrap(reply.iops_rd)); +PyDict_SetItem(ret, libvirt_constcharPtrWrap(iops_wr), + libvirt_intWrap(reply.iops_wr)); +return ret; +} + /*** * Helper functions to avoid importing modules * for every callback @@ -4739,6 +4781,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) virDomainSnapshotListNames, libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, {(char *) virDomainRevertToSnapshot, libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, {(char *) virDomainGetBlockJobInfo, libvirt_virDomainGetBlockJobInfo, METH_VARARGS, NULL}, +{(char *) virDomainGetBlockIoThrottle, libvirt_virDomainBlockIoThrottle, METH_VARARGS, NULL}, {(char *) virDomainSendKey, libvirt_virDomainSendKey, METH_VARARGS, NULL}, {(char *) virDomainMigrateGetMaxSpeed, libvirt_virDomainMigrateGetMaxSpeed, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 3/5] Implement virDomainBlockIoThrottle for the qemu driver
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- src/qemu/qemu_command.c | 35 ++ src/qemu/qemu_driver.c | 54 + src/qemu/qemu_migration.c| 16 +++--- src/qemu/qemu_monitor.c | 19 +++ src/qemu/qemu_monitor.h |6 ++ src/qemu/qemu_monitor_json.c | 107 ++ src/qemu/qemu_monitor_json.h |6 ++ src/qemu/qemu_monitor_text.c | 61 src/qemu/qemu_monitor_text.h |6 ++ 9 files changed, 302 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cf99f89..c4d2938 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1728,6 +1728,41 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } } +/*block I/O throttling*/ +if (disk-blkiothrottle.bps || disk-blkiothrottle.bps_rd +|| disk-blkiothrottle.bps_wr || disk-blkiothrottle.iops +|| disk-blkiothrottle.iops_rd || disk-blkiothrottle.iops_wr) { +if (disk-blkiothrottle.bps) { +virBufferAsprintf(opt, ,bps=%llu, + disk-blkiothrottle.bps); +} + +if (disk-blkiothrottle.bps_rd) { +virBufferAsprintf(opt, ,bps_wr=%llu, + disk-blkiothrottle.bps_rd); +} + +if (disk-blkiothrottle.bps_wr) { +virBufferAsprintf(opt, ,bps_wr=%llu, + disk-blkiothrottle.bps_wr); +} + +if (disk-blkiothrottle.iops) { +virBufferAsprintf(opt, ,iops=%llu, + disk-blkiothrottle.iops); +} + +if (disk-blkiothrottle.iops_rd) { +virBufferAsprintf(opt, ,iops_rd=%llu, + disk-blkiothrottle.iops_rd); +} + +if (disk-blkiothrottle.iops_wr) { +virBufferAsprintf(opt, ,iops_wr=%llu, + disk-blkiothrottle.iops_wr); +} +} + if (virBufferError(opt)) { virReportOOMError(); goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5588d93..bbee9a3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10449,6 +10449,59 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, return ret; } +static int +qemuDomainBlockIoThrottle(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +%s, _(domain is not running)); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +priv = vm-privateData; +ret = qemuMonitorBlockIoThrottle(priv-mon, device, info, reply, flags); +qemuDomainObjExitMonitorWithDriver(driver, vm); +if (qemuDomainObjEndJob(driver, vm) == 0) { +vm = NULL; +goto cleanup; +} + +cleanup: +VIR_FREE(device); +if (vm) +virDomainObjUnlock(vm); +qemuDriverUnlock(driver); +return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU, @@ -10589,6 +10642,7 @@ static virDriver qemuDriver = { .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */ +.domainBlockIoThrottle = qemuDomainBlockIoThrottle, /* 0.9.4 */ }; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4516231..b932ef5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1394,9 +1394,9 @@ struct _qemuMigrationSpec { #define TUNNEL_SEND_BUF_SIZE 65536 -typedef struct _qemuMigrationIOThread qemuMigrationIOThread; -typedef qemuMigrationIOThread *qemuMigrationIOThreadPtr; -struct _qemuMigrationIOThread { +typedef struct _qemuMigrationIoThread qemuMigrationIoThread; +typedef qemuMigrationIoThread *qemuMigrationIoThreadPtr; +struct _qemuMigrationIoThread { virThread thread; virStreamPtr
Re: [libvirt] [libvirt-glib] API to deal with storage pool(s)
On 09/27/2011 06:19 AM, Zeeshan Ali (Khattak) wrote: From: Zeeshan Ali (Khattak)zeesha...@gnome.org Add API to fetch, list, retrieve find storage pool(s) on a connection. --- libvirt-gobject/libvirt-gobject-connection.c | 279 ++ libvirt-gobject/libvirt-gobject-connection.h | 12 +- libvirt-gobject/libvirt-gobject.sym |6 + 3 files changed, 296 insertions(+), 1 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 69c6956..c512e79 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -43,6 +43,7 @@ struct _GVirConnectionPrivate virConnectPtr conn; GHashTable *domains; +GHashTable *pools; }; G_DEFINE_TYPE(GVirConnection, gvir_connection, G_TYPE_OBJECT); @@ -357,6 +358,11 @@ void gvir_connection_close(GVirConnection *conn) priv-domains = NULL; } +if (priv-pools) { +g_hash_table_unref(priv-pools); +priv-pools = NULL; +} + if (priv-conn) { virConnectClose(priv-conn); priv-conn = NULL; @@ -503,6 +509,148 @@ cleanup: return ret; } +/** + * gvir_connection_fetch_storage_pools: + * @conn: the connection + * @cancellable: (allow-none)(transfer none): cancellation object + */ +gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, + GCancellable *cancellable, + GError **err) +{ +GVirConnectionPrivate *priv = conn-priv; +GHashTable *pools; +gchar **inactive = NULL; +gint ninactive = 0; +gchar **active = NULL; +gint nactive = 0; +gboolean ret = FALSE; +gint i; +virConnectPtr vconn = NULL; + +g_mutex_lock(priv-lock); +if (!priv-conn) { +*err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + Connection is not open); +g_mutex_unlock(priv-lock); +goto cleanup; +} +vconn = priv-conn; +/* Stop another thread closing the connection just at the minute */ +virConnectRef(vconn); +g_mutex_unlock(priv-lock); + +if (g_cancellable_set_error_if_cancelled(cancellable, err)) +goto cleanup; + +if ((nactive = virConnectNumOfStoragePools(vconn)) 0) { +*err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + Unable to count pools); +goto cleanup; +} +if (nactive) { +if (g_cancellable_set_error_if_cancelled(cancellable, err)) +goto cleanup; + +active = g_new(gchar *, nactive); +if ((nactive = virConnectListStoragePools(vconn, + active, + nactive)) 0) { +*err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + Unable to list pools); +goto cleanup; +} +} + +if (g_cancellable_set_error_if_cancelled(cancellable, err)) +goto cleanup; + +if ((ninactive = virConnectNumOfDefinedStoragePools(vconn)) 0) { +*err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + Unable to count pools); +goto cleanup; +} + +if (ninactive) { +if (g_cancellable_set_error_if_cancelled(cancellable, err)) +goto cleanup; + +inactive = g_new(gchar *, ninactive); +if ((ninactive = virConnectListDefinedStoragePools(vconn, + inactive, + ninactive)) 0) { +*err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + Unable to list pools %d, ninactive); +goto cleanup; +} +} + +pools = g_hash_table_new_full(g_str_hash, + g_str_equal, + g_free, + g_object_unref); + +for (i = 0 ; i nactive ; i++) { +if (g_cancellable_set_error_if_cancelled(cancellable, err)) +goto cleanup; + +virStoragePoolPtr vpool; +GVirStoragePool *pool; + +vpool = virStoragePoolLookupByName(vconn, active[i]); +if (!vpool) +continue; + +pool = GVIR_STORAGE_POOL(g_object_new(GVIR_TYPE_STORAGE_POOL, + handle, vpool, + NULL)); + +g_hash_table_insert(pools, +g_strdup(gvir_storage_pool_get_uuid(pool)), +pool); +} + +for (i = 0 ; i ninactive ; i++) { +if (g_cancellable_set_error_if_cancelled(cancellable, err))
Re: [libvirt] [PATCH v2] qemu: Allow domain reboot after core dump
On 09/21/2011 02:47 PM, Michal Privoznik wrote: This patch introduces possibility to reboot domain after core dump finishes. The new flag VIR_DUMP_REBOOT was added to virDomainCoreDumpFlags. The new functionality is accessible via virsh too: virsh dump --rebootdomain --- diff to v1: -check for success of reboot include/libvirt/libvirt.h.in |1 + src/qemu/qemu_driver.c |9 - tools/virsh.c|3 +++ 3 files changed, 12 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 39155a6..8c41f5a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -748,6 +748,7 @@ typedef enum { VIR_DUMP_CRASH= (1 0), /* crash after dump */ VIR_DUMP_LIVE = (1 1), /* live dump */ VIR_DUMP_BYPASS_CACHE = (1 2), /* avoid file system cache pollution */ +VIR_DUMP_REBOOT = (1 3), /* reboot domain after dump finishes */ } virDomainCoreDumpFlags; /* Domain migration flags. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f4ee4c3..f040384 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3104,7 +3104,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; -virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH | VIR_DUMP_BYPASS_CACHE, -1); +virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH | + VIR_DUMP_BYPASS_CACHE | VIR_DUMP_REBOOT, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); @@ -3189,6 +3190,12 @@ cleanup: if (event) qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); + +if ((ret == 0) (flags VIR_DUMP_REBOOT) +(qemuDomainReboot(dom, 0) 0)) { +ret = -1; +} + return ret; } diff --git a/tools/virsh.c b/tools/virsh.c index 371346a..deadb4e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2899,6 +2899,7 @@ static const vshCmdOptDef opts_dump[] = { {crash, VSH_OT_BOOL, 0, N_(crash the domain after core dump)}, {bypass-cache, VSH_OT_BOOL, 0, N_(avoid file system cache when saving)}, +{reboot, VSH_OT_BOOL, 0, N_(reboot the domain after core dump)}, {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, {file, VSH_OT_DATA, VSH_OFLAG_REQ, N_(where to dump the core)}, {NULL, 0, 0, NULL} @@ -2928,6 +2929,8 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DUMP_CRASH; if (vshCommandOptBool(cmd, bypass-cache)) flags |= VIR_DUMP_BYPASS_CACHE; +if (vshCommandOptBool(cmd, reboot)) +flags |= VIR_DUMP_REBOOT; if (virDomainCoreDump(dom, to, flags) 0) { vshError(ctl, _(Failed to core dump domain %s to %s), name, to); I met some error like below when I test virsh command virsh dump -reboot: virsh # start kvm-one Domain kvm-one started virsh # dump --reboot kvm-one /home/lei/dumpcore error: Failed to core dump domain kvm-one to /home/lei/dumpcore error: Unable to read from monitor: Connection reset by peer virsh # list Id Name State -- virsh # After dump --reboot, my domain shutdown unexpectedly. Did I miss anything? -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Source control for storage pool
Fix bug #611823 storage driver should prohibit pools with duplicate underlying storage. Add API virStoragePoolSourceFindDuplicate() to do uniqueness check based on source location infomation for pool type. Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 80 ++ src/conf/storage_conf.h |5 +++ src/libvirt_private.syms |2 + src/storage/storage_driver.c |6 +++ 4 files changed, 93 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8d14e87..1e7da69 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1311,6 +1311,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool, + virStoragePoolDefPtr def) { +unsigned int i, j; + +for (i = 0; i pool-def-source.ndevice; i++) { +for (j = 0; j def-source.ndevice; j++) { +if (STREQ(pool-def-source.devices[i].path, def-source.devices[j].path)) +return pool; +} +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1701,6 +1716,71 @@ cleanup: return ret; } +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int i; +int ret = 1; +virStoragePoolObjPtr pool = NULL; +virStoragePoolObjPtr matchpool = NULL; + +/* Check the pool list for duplicate underlying storage */ +for (i = 0; i pools-count; i++) { +pool = pools-objs[i]; +if (def-type != pool-def-type) +continue; + +virStoragePoolObjLock(pool); + +switch (pool-def-type) { +case VIR_STORAGE_POOL_DIR: +if (STREQ(pool-def-target.path, def-target.path)) +matchpool = pool; +break; +case VIR_STORAGE_POOL_NETFS: +if ((STREQ(pool-def-source.dir, def-source.dir)) \ + (STREQ(pool-def-source.host.name, def-source.host.name))) +matchpool = pool; +break; +case VIR_STORAGE_POOL_SCSI: +if (STREQ(pool-def-source.adapter, def-source.adapter)) +matchpool = pool; +break; +case VIR_STORAGE_POOL_ISCSI: +{ +matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); +if (matchpool) { +if (STREQ(matchpool-def-source.host.name, def-source.host.name)) { +if ((matchpool-def-source.initiator.iqn) (def-source.initiator.iqn)) { +if (STREQ(matchpool-def-source.initiator.iqn, def-source.initiator.iqn)) +break; +matchpool = NULL; +} +break; +} +matchpool = NULL; +} +break; +} +case VIR_STORAGE_POOL_FS: +case VIR_STORAGE_POOL_LOGICAL: +case VIR_STORAGE_POOL_DISK: +matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); +break; +default: +break; +} +virStoragePoolObjUnlock(pool); +} + +if (matchpool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(Storage source conflict with pool: '%s'), + matchpool-def-name); +ret = -1; +} +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..d115a15 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool, + virStoragePoolDefPtr def); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); @@ -388,6 +390,9 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); + void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src
[libvirt] [PATCH] Source control for storage pool
Fix bug #611823 storage driver should prohibit pools with duplicate underlying storage. Add API virStoragePoolSourceFindDuplicate() to do uniqueness check based on source location infomation for pool type. Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 73 ++ src/conf/storage_conf.h |3 ++ src/libvirt_private.syms |1 + src/storage/storage_driver.c |6 +++ 4 files changed, 83 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8d14e87..698334e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1701,6 +1701,79 @@ cleanup: return ret; } +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int i, j, k; +int ret = 1; +virStoragePoolObjPtr pool = NULL; +virStoragePoolObjPtr matchpool = NULL; + +/* Check the pool list for duplicate underlying storage */ +for (i = 0; i pools-count; i++) { +pool = pools-objs[i]; +if (def-type != pool-def-type) +continue; + +virStoragePoolObjLock(pool); +if (pool-def-type == VIR_STORAGE_POOL_DIR) { +if (STREQ(pool-def-target.path, def-target.path)) { +matchpool = pool; +goto cleanup; +} +} else if (pool-def-type == VIR_STORAGE_POOL_NETFS) { +if ((STREQ(pool-def-source.dir, def-source.dir)) \ + (STREQ(pool-def-source.host.name, def-source.host.name))) { +matchpool = pool; +goto cleanup; +} +} else if (pool-def-type == VIR_STORAGE_POOL_SCSI) { +if (STREQ(pool-def-source.adapter, def-source.adapter)) { +matchpool = pool; +goto cleanup; +} +} else if (pool-def-type == VIR_STORAGE_POOL_ISCSI) { +for (j = 0; j pool-def-source.ndevice; j++) { +for (k = 0; k def-source.ndevice; k++) { +if (pool-def-source.initiator.iqn) { +if ((STREQ(pool-def-source.devices[j].path, def-source.devices[k].path)) \ + (STREQ(pool-def-source.initiator.iqn, def-source.initiator.iqn))) { +matchpool = pool; +goto cleanup; +} +} else if (pool-def-source.host.name) { +if ((STREQ(pool-def-source.devices[j].path, def-source.devices[k].path)) \ + (STREQ(pool-def-source.host.name, def-source.host.name))) { +matchpool = pool; +goto cleanup; +} +} +} +} +} else { +/* For the remain three pool type 'fs''logical''disk' that all use device path */ +if (pool-def-source.ndevice) { +for (j = 0; j pool-def-source.ndevice; j++) { +for (k = 0; k def-source.ndevice; k++) { +if (STREQ(pool-def-source.devices[j].path, def-source.devices[k].path)) { +matchpool = pool; +goto cleanup; +} +} +} +} +} +virStoragePoolObjUnlock(pool); +} +cleanup: +if (matchpool) { + virStoragePoolObjUnlock(matchpool); + virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(pool source location info is duplicate)); + ret = -1; +} +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..a8562a6 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -388,6 +388,9 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); + void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f03e30..5899d56 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -948,6 +948,7 @@ virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; virStoragePoolObjIsDuplicate; +virStoragePoolSourceFindDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 68cac1f..c05b74e 100644 --- a/src/storage
[libvirt] Draft document about source control for type of pools
Hi Daniel, The last version of my patch fixed bug #611823 prohibit pools with duplicate storage, you gave some comments that should do check by source info instead of target path I used. These days I view the code and existing documents about storage pool, and tried to code based on your comments, but looks like it is turning out to be more complex. First, I think it is possible to make clear what it means for a pool to be a duplicate. Below is the detailed source info be used for each type of pool according to the existing documents and the main item to control. For details of each type of pool: VIR_STORAGE_POOL_DIR, /* For dir type, use source-dir only(But in the code and example XML file,it use target path).*/ A pool with a type of dir provides the means to manage files within a directory. VIR_STORAGE_POOL_FS, /* For fs type, use source-device-path.*/ This is a variant of the directory pool. Instead of creating a directory on an existing mounted filesystem though, it expects a source block device to be named. This block device will be mounted and files managed in the directory of its mount point. VIR_STORAGE_POOL_NETFS,/* For netfs, use host name and source-dir */ This is a variant of the filesystem pool. Instead of requiring a local block device as the source, it requires the name of a host and path of an exported directory. It will mount this network filesystem and manage files within the directory of its mount point. VIR_STORAGE_POOL_LOGICAL, /* For logical, use source-device-path. */ This provides a pool based on an LVM volume group. For a pre-defined LVM volume group, simply providing the group name is sufficient,while to build a new group requires providing a list of source devices to serve as physical volumes. VIR_STORAGE_POOL_DISK, /* For disk type, use source-device-path.*/ This provides a pool based on a physical disk. Volumes are created by adding partitions to the disk. VIR_STORAGE_POOL_ISCSI,/* For ISCSI type, use source-device-path. */ This provides a pool based on an iSCSI target. VIR_STORAGE_POOL_SCSI, /* For SCSI type, use source-adapter name. */ This provides a pool based on a SCSI HBA. VIR_STORAGE_POOL_MPATH, This provides a pool that contains all the multipath devices on the host. Volume creating is not supported via the libvirt APIs, so we will just ignore this type. After a preliminary investigation, for source element, source-device-path, -Pool type: fs, logical, disk, ISCSI. -May only occur once. source-dir, -Pool type: dir, netfs. -May only occur once. source-adapter -Pool type: SCSI. -May only occur once. are the main required location info, and we can control the duplicate storage based on these three item above for related type of pool. Second, I have tried to do check by source info for a test, when I debug, I found that pools-objs[]-def-source.* which come from virConnectPtr where all existing pools info be keeped == NULL, but the current target pool virStoragePoolDefPtr def can get source field value, so I think libvirt didn't get source field info in the pools list where we do check at all today. Do you think we should do check based on this? Or any comments for question one and two. After you confirmed then I will work on code. Thank you. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libvirt] [PATCH v2] Fix bug #611823 prohibit pools with duplicate storage
On 08/03/2011 05:29 AM, Daniel P. Berrange wrote: On Wed, Aug 03, 2011 at 12:52:50AM +0800, Lei Li wrote: On 08/02/2011 07:11 PM, Daniel P. Berrange wrote: On Mon, Aug 01, 2011 at 02:12:51PM -0600, Eric Blake wrote: On 07/31/2011 10:58 PM, Lei Li wrote: Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Wrap your commit messages; typically at 70 columns or so (since 'git log' adds some indentation, but you want the end result to still fit in 80 columns for legibility). Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 48 insertions(+), 0 deletions(-) +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; ipools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} This one is good; in fact, we may even want to expose it as a public API, parallel to other virStoragePoolLookupBy* functions (but not until after 0.9.4 is released) No, this API is flawed because def-target.path is not required to be unique for all types of storage pool. Daniel Yes, in the beginning it seems like target-path is not required to be unique. But for this bug https://bugzilla.redhat.com/show_bug.cgi?id=611823 you reported, you said that For example, if two directory pools point to the same directory, and one pool is used to create a volume, the other pool will remain unaware of the new volume until it is refreshed. And I have test it when use 'virsh pool-define/create' it will create more than two pools not two have the same directory. I think maybe you should look at the description of the bug first. This API virStoragePoolObjFindByPath() just provide a method to search pool obj by path and can be use to avoid duplicate target path to fix this bug you mentioned. Ah a slight misunderstanding here. There are quite a few different pieces of metadata with storage pools, and in some cases they are the same. - name/uuid - usual unique metadata for a storage pool - source - the data for the source varies according to storage pool type - hostname - directory path - adaptor - device path - source name - initiator IQN - target - a path that controls how storage volume paths are formed I think your misunderstanding is because for 'directory' storage pools, the source directory path is actually the same as the target path. So if you want to do uniqueness checking for storage pools, you need todo it based on the source information, rather than the target path. The checks will of course need to be slightly different for each storage pool type. Regards, Daniel Hi Daniel, I seriously considered your comments and look at the document about storage pool and volume XML format. Yes, there are kinds type of pools, but the main goal of the bug #611823 you reported is to avoid an inconsistent view of it's volume created on the same pool. The source directory info for each type of pool maybe different, but if the target.path controls how storage volume paths are formed, why should we just check the target.path to avoid this issue? -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libvirt] [PATCH v2] Fix bug #611823 prohibit pools with duplicate storage
On 08/02/2011 07:11 PM, Daniel P. Berrange wrote: On Mon, Aug 01, 2011 at 02:12:51PM -0600, Eric Blake wrote: On 07/31/2011 10:58 PM, Lei Li wrote: Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Wrap your commit messages; typically at 70 columns or so (since 'git log' adds some indentation, but you want the end result to still fit in 80 columns for legibility). Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 48 insertions(+), 0 deletions(-) +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} This one is good; in fact, we may even want to expose it as a public API, parallel to other virStoragePoolLookupBy* functions (but not until after 0.9.4 is released) No, this API is flawed because def-target.path is not required to be unique for all types of storage pool. Daniel Yes, in the beginning it seems like target-path is not required to be unique. But for this bug https://bugzilla.redhat.com/show_bug.cgi?id=611823 you reported, you said that For example, if two directory pools point to the same directory, and one pool is used to create a volume, the other pool will remain unaware of the new volume until it is refreshed. And I have test it when use 'virsh pool-define/create' it will create more than two pools not two have the same directory. I think maybe you should look at the description of the bug first. This API virStoragePoolObjFindByPath() just provide a method to search pool obj by path and can be use to avoid duplicate target path to fix this bug you mentioned. BTW, I found that there are 3 method provide ways to search by 'key','name','path' in storage volume also. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libvirt] [PATCH v2] Fix bug #611823 prohibit pools with duplicate storage
On 08/02/2011 07:11 PM, Daniel P. Berrange wrote: On Mon, Aug 01, 2011 at 02:12:51PM -0600, Eric Blake wrote: On 07/31/2011 10:58 PM, Lei Li wrote: Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Wrap your commit messages; typically at 70 columns or so (since 'git log' adds some indentation, but you want the end result to still fit in 80 columns for legibility). Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 48 insertions(+), 0 deletions(-) +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} This one is good; in fact, we may even want to expose it as a public API, parallel to other virStoragePoolLookupBy* functions (but not until after 0.9.4 is released) No, this API is flawed because def-target.path is not required to be unique for all types of storage pool. Daniel And you said in the bug description that The simplest example is two directory pools that point to the same directory, but iSCSI and other pool types behave similarly. Based on your description, step to reproduce and expected results, I look at the code about process of storage pool, I agree with your conclusion. But now I was confused for your comment target.path is not required to be unique for all types of storage pool. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [Libvirt] [PATCH v3] Fix bug #611823 prohibit pools with duplicate storage
Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 25 + src/conf/storage_conf.h |2 ++ src/libvirt_private.syms |1 + 3 files changed, 28 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..1d9fe25 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1317,6 +1317,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1700,6 +1715,16 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, } } +/* Check the pool list if defined target path already exist */ +pool = virStoragePoolObjFindByPath(pools, def-target.path); +if (pool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(target path '%s' is already in use), + pool-def-target.path); +dupPool = -1; +goto cleanup; +} + ret = dupPool; cleanup: if (pool) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..9239977 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..19f5f92 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -937,6 +937,7 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [Libvirt] [PATCH v2] Fix bug #611823 prohibit pools with duplicate storage
Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 48 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..9078f78 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1317,6 +1317,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1707,6 +1722,27 @@ cleanup: return ret; } +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int ret = 1; +virStoragePoolObjPtr pool = NULL; + +/* Check the pool list if defined target path already exist */ +pool = virStoragePoolObjFindByPath(pools, def-target.path); +if (pool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(target path '%s' is already in use), + pool-def-target.path); +ret = -1; +goto cleanup; +} + +cleanup: +if (pool) +virStoragePoolObjUnlock(pool); +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..454c43d 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); @@ -387,6 +389,8 @@ char *virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def); int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..37afaf2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -937,7 +937,9 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; +virStoragePoolTargetDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9c353e3..b757911 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 1) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if ((backend = virStorageBackendForType(def-type)) == NULL) goto cleanup; @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 0) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if (virStorageBackendForType(def-type) == NULL) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix bug#611823 prohibit pools with duplicate storage
Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |5 + src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 49 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..eb3595c 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1317,6 +1317,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1707,6 +1722,27 @@ cleanup: return ret; } +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int ret = 1; +virStoragePoolObjPtr pool = NULL; + +/* Check the pool list if defined target path already exist */ +pool = virStoragePoolObjFindByPath(pools, def-target.path); +if (pool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(target path '%s' is already in use), + pool-def-target.path); +ret = -1; +goto cleanup; +} + +cleanup: +if (pool) +virStoragePoolObjUnlock(pool); +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..44a2cc8 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); @@ -388,6 +390,9 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); + void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..37afaf2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -937,7 +937,9 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; +virStoragePoolTargetDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9c353e3..b757911 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 1) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if ((backend = virStorageBackendForType(def-type)) == NULL) goto cleanup; @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 0) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if (virStorageBackendForType(def-type) == NULL) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix bug #611823 prohibit pools with duplicate storage
Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created. Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 36 src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 48 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..9078f78 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1317,6 +1317,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) { +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); +if (STREQ(pools-objs[i]-def-target.path, path)) +return pools-objs[i]; +virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1707,6 +1722,27 @@ cleanup: return ret; } +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int ret = 1; +virStoragePoolObjPtr pool = NULL; + +/* Check the pool list if defined target path already exist */ +pool = virStoragePoolObjFindByPath(pools, def-target.path); +if (pool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(target path '%s' is already in use), + pool-def-target.path); +ret = -1; +goto cleanup; +} + +cleanup: +if (pool) +virStoragePoolObjUnlock(pool); +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..454c43d 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); @@ -387,6 +389,8 @@ char *virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def); int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..37afaf2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -937,7 +937,9 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; +virStoragePoolTargetDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9c353e3..b757911 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 1) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if ((backend = virStorageBackendForType(def-type)) == NULL) goto cleanup; @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 0) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if (virStorageBackendForType(def-type) == NULL) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix bug #611823 prohibit pools with duplicate storage
To make sure the unique storage pool defined and created from different directory to avoid inconsistent version of volume pool created, I add two API be called by storage driver to check for the probable duplicate pools and refused the duplicate pool. virStoragePoolObjFindByPath() provide a method to find pool object by target path in pool list. virStoragePoolTargetDuplicate() implement the function to check if there is duplicate pool. Add judgement for storagePoolCreatestoragePoolDefine by calling virStoragePoolTargetDuplicate() to avoid both transient storage pool and persistent storage pool be created repeatedly in storage driver. Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- src/conf/storage_conf.c | 39 +++ src/conf/storage_conf.h |4 src/libvirt_private.syms |2 ++ src/storage/storage_driver.c |6 ++ 4 files changed, 51 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..a499e82 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1317,6 +1317,22 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, return NULL; } +virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, +const char *path) +{ +unsigned int i; + +for (i = 0 ; i pools-count ; i++) { +virStoragePoolObjLock(pools-objs[i]); + if (STREQ(pools-objs[i]-def-target.path, path)) + return pools-objs[i]; + virStoragePoolObjUnlock(pools-objs[i]); +} + +return NULL; +} + void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { @@ -1707,6 +1723,29 @@ cleanup: return ret; } +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ +int ret = 1; +virStoragePoolObjPtr pool = NULL; + +/*check pool list if defined target path already exist*/ +pool = virStoragePoolObjFindByPath(pools, def-target.path); + +if (pool) { +virStorageReportError(VIR_ERR_OPERATION_FAILED, + _(target path '%s' is already in use), + pool-def-target.path); +ret = -1; +goto cleanup; +} + + +cleanup: +if (pool) +virStoragePoolObjUnlock(pool); +return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..454c43d 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +virStoragePoolObjPtr virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key); @@ -387,6 +389,8 @@ char *virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def); int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 853ee62..ef323f5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -930,7 +930,9 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; +virStoragePoolTargetDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9c353e3..8ee63f6 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 1) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if ((backend = virStorageBackendForType(def-type)) == NULL) goto cleanup; @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(driver-pools, def, 0) 0) goto cleanup; +if (virStoragePoolTargetDuplicate(driver-pools, def) 0) +goto cleanup; + if