Re: [libvirt] [PATCH] qemu: read backing chain names from qemu
On 11/03/15 15:03 -0600, Eric Blake wrote: Adam, since you reported the issue, let me know if you need a scratch binary to test this patch on. Jeff, thanks for the idea on how to solve the problem without touching qemu (of course, it would still be nice to teach qemu to respect inode equivalence rather than relying on streq, and even nicer for libvirt to use node names instead of file names, but those can be later patches without holding up VDSM now). Thanks Eric. I am in the process of doing a crude port to RHEL-7.1 (libvirt-1.2.8-16) in order to verify it, but a proper build would probably be best (when you have some time). -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ideas on virDomainListBlockStats for allocation numbers
... block.1.name=vda[1] # information on backingStore index 1 of vda block.1.rd.reqs=0 #+ that is, on /dev/sda6 ... block.2.name=vdb # information on /dev/sda7 ... It may make things easier if I also add a block.n.path that lists the file name of the block being described (might get tricky with NBD/gluster/sheepdog network disks). Also, I'm thinking of adding block.n.physical to match the older virDomainGetBlockInfo() information. Another possible layout is to mirror the nesting of XML backingChain. Something like: block.count=2 block.0.name=vda block.0.backing=1 block.0.allocation=... # information on /tmp/wrapper.qcow2 ... block.0.0.allocation=... # information on /dev/sda6 ... block.1.name=vdb block.1.backing=0 block.1.allocation=... # information on /dev/sda7 But there, we run into a possible problem: virTypedParameter has a finite length for field names, so we can only cover a finite depth of backing chain before we run out of space and can't report on the full chain. Any other ideas for the best way to lay this out, and for how to make it as easy as possible for client applications to correlate information on allocation back to the appropriate block device in the chain? I also wonder if adding more information to the existing --block flag for stats is okay, or whether it is better to add yet another statistics flag grouping that must be requested to turn on information about backing files. Technically, it's still block-related statistics, but as we have already got released libvirt that still has a 1:1 block.n mapping to disk elements, using a new flag would make it easier to learn if libvirt is new enough to support information on backing chains, all without confusing existing clients that aren't expecting backing chain stats. This question needs answering regardless of which layout we choose above for representing backing chain stats. Thoughts welcome. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/7] dumpxml: let blockinfo reuse virStorageSource
On 18/11/14 06:31 -0700, Eric Blake wrote:
In a future patch, the implementation of VIR_DOMAIN_XML_BLOCK_INFO
will use information stored in virStorageSource. In order to
maximize code reuse, it is easiest if BlockInfo code also uses the
same location for information.
* src/util/virstoragefile.h (_virStorageSource): Add physical, to
mirror virDomainBlockInfo.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
storage source, then copy to block info.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/qemu/qemu_driver.c| 22 ++
src/util/virstoragefile.h | 3 ++-
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cd70fde..aa24658 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11046,15 +11046,15 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
/* Get info for normal formats */
if (S_ISREG(sb.st_mode) || fd == -1) {
#ifndef WIN32
-info-physical = (unsigned long long)sb.st_blocks *
+disk-src-physical = (unsigned long long)sb.st_blocks *
(unsigned long long)DEV_BSIZE;
#else
-info-physical = sb.st_size;
+disk-src-physical = sb.st_size;
#endif
/* Regular files may be sparse, so logical size (capacity) is not same
* as actual physical above
*/
-info-capacity = sb.st_size;
+disk-src-capacity = sb.st_size;
} else {
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
@@ -11065,17 +11065,17 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
_(failed to seek to end of %s), path);
goto endjob;
}
-info-physical = end;
-info-capacity = end;
+disk-src-physical = end;
+disk-src-capacity = end;
}
/* If the file we probed has a capacity set, then override
* what we calculated from file/block extents */
if (meta-capacity)
-info-capacity = meta-capacity;
+disk-src-capacity = meta-capacity;
/* Set default value .. */
-info-allocation = info-physical;
+disk-src-allocation = disk-src-physical;
/* ..but if guest is not using raw disk format and on a block device,
* then query highest allocated extent from QEMU
@@ -11097,13 +11097,19 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockExtent(priv-mon,
disk-info.alias,
-info-allocation);
+disk-src-allocation);
qemuDomainObjExitMonitor(driver, vm);
} else {
ret = 0;
}
+if (ret == 0) {
+info-capacity = disk-src-capacity;
+info-allocation = disk-src-allocation;
+info-physical = disk-src-physical;
+}
+
endjob:
if (!qemuDomainObjEndJob(driver, vm))
vm = NULL;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 2583e10..681e50a 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -252,8 +252,9 @@ struct _virStorageSource {
virStoragePermsPtr perms;
virStorageTimestampsPtr timestamps;
-unsigned long long allocation; /* in bytes, 0 if unknown */
unsigned long long capacity; /* in bytes, 0 if unknown */
+unsigned long long allocation; /* in bytes, 0 if unknown */
(Pedantic and superficial) allocation dropped and re-added. Maybe you
prefer it to appear after capacity in light of physical now being
present?
+unsigned long long physical; /* in bytes, 0 if unknown */
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
--
1.9.3
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/7] dumpxml: add flag to virDomainGetXMLDesc
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
*
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
* error. The caller must free() the returned value.
@@ -2592,6 +2593,11 @@ virDomainGetControlInfo(virDomainPtr domain,
* describing CPU capabilities is modified to match actual
* capabilities of the host.
*
+ * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
+ * disk device will contain additional information such as capacity
+ * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(),
+ * and avoiding the need to call virDomainGetBlockInfo() separately.
+ *
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
*/
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a7e9151..4c63006 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8984,6 +8984,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
.type = VSH_OT_BOOL,
.help = N_(provide XML suitable for migrations)
},
+{.name = block-info,
+ .type = VSH_OT_BOOL,
+ .help = N_(include additional block info for each disk in XML dump)
+},
{.name = NULL}
};
@@ -9007,6 +9011,8 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_XML_UPDATE_CPU;
if (migratable)
flags |= VIR_DOMAIN_XML_MIGRATABLE;
+if (vshCommandOptBool(cmd, block-info))
+flags |= VIR_DOMAIN_XML_BLOCK_INFO;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index d5608cc..28f928f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1290,7 +1290,7 @@ NOTE: Some hypervisors may require the user to manually
ensure proper
permissions on file and path specified by argument Icorefilepath.
=item Bdumpxml Idomain [I--inactive] [I--security-info]
-[I--update-cpu] [I--migratable]
+[I--update-cpu] [I--migratable] [I--block-info]
Output the domain information as an XML dump to stdout, this format can be used
by the Bcreate command. Additional options affecting the XML dump may be
@@ -1301,7 +1301,9 @@ in the XML dump. I--update-cpu updates domain CPU
requirements according to
host CPU. With I--migratable one can request an XML that is suitable for
migrations, i.e., compatible with older libvirt releases and possibly amended
with internal run-time options. This option may automatically enable other
-options (I--update-cpu, I--security-info, ...) as necessary.
+options (I--update-cpu, I--security-info, ...) as necessary. Using
+I--block-info will add additional information about each disk source,
+comparable to what Bvol-dumpxml or Bdomblkinfo would show.
=item Bedit Idomain
--
1.9.3
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/7] dumpxml: add flag to virDomainGetXMLDesc
On 18/11/14 09:33 -0700, Eric Blake wrote:
On 11/18/2014 07:50 AM, Adam Litke wrote:
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index 1fac2a3..caf1f46 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1248,6 +1248,7 @@ typedef enum {
VIR_DOMAIN_XML_INACTIVE = (1 1), /* dump inactive domain
information */
VIR_DOMAIN_XML_UPDATE_CPU = (1 2), /* update guest CPU
requirements according to host CPU */
VIR_DOMAIN_XML_MIGRATABLE = (1 3), /* dump XML suitable for
migration */
+VIR_DOMAIN_XML_BLOCK_INFO = (1 4), /* include storage volume
information about each disk source */
I'll admit I'm not a master API designer but this is a red flag for me
(pun most definitely intended). Up to this point, the individual
virDomainXMLFlags are used to ask for XML for a certain purpose. For
example, VIR_DOMAIN_XML_INACTIVE retrieves XML suitable for a call to
virDomainCreateXML whereas VIR_DOMAIN_XML_MIGRATABLE is used to dump
XML suitable for migration.
Not quite. The VIR_DOMAIN_XML_SECURE and VIR_DOMAIN_XML_UPDATE_CPU
flags both have the behavior of dumping additional information that is
omitted by default. And the VIR_DOMAIN_XML_INACTIVE flag really means
'dump the xml that will be used on the next boot' if the domain is
persistent (if the domain is transient, it says 'dump the xml that would
be used on the next boot if I make the domain persistent').
This new flag is somewhat arbitrarily slicing up some of the ACTIVE VM
information. Slippery slope logic would lead us to a future API with
a proliferation of flags that turn various bits of info on and off
which would be very cumbersome to maintain and use. Since this
information is really ACTIVE VM info, I'd prefer it to be provided
whenever VIR_DOMAIN_XML_INACTIVE is not set. Callers who want a
tight XML document can always use VIR_DOMAIN_XML_INACTIVE.
The thing is that this new flag is providing additional informative
information that is NOT being stored as part of the domain itself, but
which is being queried from the disks each time the xml is gathered.
What's more, the information is valid for both active and inactive xml
(it is not an active-only flag), and gathering the information
potentially requires a monitor command for an active domain (none of the
other XML information requires a monitor command). It is not good to
require a monitor command by default, which is why I felt that having
the flag opt-in to the behavior is the best way to preserve back-compat
for callers that don't need the extra information.
Then perhaps the right flag to add would be VIR_DOMAIN_XML_QUERY or
some such to indicate that extra data needs to be retrieved via
potentially expensive means. Then, the flag (and its semantics) could
be reused in the future.
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/7] dumpxml: let blockinfo reuse virStorageSource
On 18/11/14 09:35 -0700, Eric Blake wrote:
On 11/18/2014 08:00 AM, Adam Litke wrote:
On 18/11/14 06:31 -0700, Eric Blake wrote:
In a future patch, the implementation of VIR_DOMAIN_XML_BLOCK_INFO
will use information stored in virStorageSource. In order to
maximize code reuse, it is easiest if BlockInfo code also uses the
same location for information.
+++ b/src/util/virstoragefile.h
@@ -252,8 +252,9 @@ struct _virStorageSource {
virStoragePermsPtr perms;
virStorageTimestampsPtr timestamps;
-unsigned long long allocation; /* in bytes, 0 if unknown */
unsigned long long capacity; /* in bytes, 0 if unknown */
+unsigned long long allocation; /* in bytes, 0 if unknown */
(Pedantic and superficial) allocation dropped and re-added. Maybe you
prefer it to appear after capacity in light of physical now being
present?
Just reordering fields so that they come in the same order as their
public struct virDomainBlockInfo counterpart in libvirt-domain.h.
I suspected there was a good reason for it :)
Order doesn't strictly matter, if you want me to avoid the churn.
Nope, please keep it the way you have it.
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/n] dumpxml: add flag to virDomainGetXMLDesc
On 17/09/14 08:43 -0600, Eric Blake wrote: + * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each + * disk device will contain additional information such as capacity + * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(), + * and avoiding the need to call virDomainGetBlockInfo() separately. + * Eric: We decided to go with this extra flag since gathering the block info was seen as a potentially expensive operation which users would want to selectively enable. Do you have any thoughts on how expensive this would be in practice? We are considering enabling it unconditionally in oVirt and am weighing a decision between A) Simplicity and Cacheability vs. B) Performance. Thanks. -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: exposing backing store allocation in domain xml
On 16/09/14 09:29 +0200, Peter Krempa wrote: On 09/12/14 23:30, Eric Blake wrote: [revisiting something that finally surfaced to the top of my todo list] On 08/07/2014 03:57 AM, Peter Krempa wrote: On 08/06/14 18:36, Eric Blake wrote: Adam Litke has been asking if I can expose watermark information from\ bikeshedding I'd be glad if we stopped calling this watermark. The wiki disambiguation article states: citation A watermark is a recognizable image or pattern in paper used to identify authenticity. Watermark or watermarking can also refer to: In digital watermarks and digital security[edit] Watermark (data file), a method for ensuring data integrity which combines aspects of data hashing and digital watermarking Watermark (data synchronization), directory synchronization related programming terminology High-water mark (computer security), We are using it in the sense of high-water mark. Etymology-wise, it goes back to the days of loading boats - you would paint a line called the high watermark; as the boat was weighed down with more cargo, you had to stop loading when that line touched the water, or risk sinking the boat. In the same vein, someone running on expandable underlying storage can let the guest consume until it hits the watermark, at which point the storage must be grown to avoid sinking the guest with an out-of-space event. But I'm open to the idea of any other terminology... For now, calling it allocation is good enough, since that is the term I will be putting in the XML. I think that allocation is fine. High-water mark might be acceptable but still looks a bit awkward to me. I also prefer 'allocation' since it describes the piece of information being presented rather than something which it might be used for. -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] blockcopy: allow block device destination
= virQEMUDriverGetConfig(driver);
@@ -15374,7 +15375,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
if (VIR_ALLOC(mirror) 0)
goto endjob;
/* XXX Allow non-file mirror destinations */
-mirror-type = VIR_STORAGE_TYPE_FILE;
+mirror-type = flags VIR_DOMAIN_BLOCK_REBASE_COPY_DEV ?
+VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE;
if (format) {
if ((mirror-format = virStorageFileFormatTypeFromString(format)) =
0) {
@@ -15466,7 +15468,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char
*path, const char *base,
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
VIR_DOMAIN_BLOCK_REBASE_COPY |
VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
- VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1);
+ VIR_DOMAIN_BLOCK_REBASE_RELATIVE |
+ VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
return -1;
@@ -15482,7 +15485,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char
*path, const char *base,
format = raw;
flags = ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
-return qemuDomainBlockCopy(vm, dom-conn, path, base, format,
bandwidth, flags);
+return qemuDomainBlockCopy(vm, dom-conn, path, base, format,
+ bandwidth, flags);
}
return qemuDomainBlockJobImpl(vm, dom-conn, path, base, bandwidth, NULL,
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index 46f2a3e..7495a45 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -17,8 +17,8 @@
disk type='block' device='disk'
source dev='/dev/HostVG/QEMUGuest1'/
backingStore/
- mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy'
ready='yes'
-source file='/dev/HostVG/QEMUGuest1Copy'/
+ mirror type='block' job='copy' ready='yes'
+source dev='/dev/HostVG/QEMUGuest1Copy'/
/mirror
target dev='hda' bus='ide'/
address type='drive' controller='0' bus='0' target='0' unit='0'/
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f7193cb..1fbd36e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1521,6 +1521,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
if (vshCommandOptBool(cmd, raw))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
+if (vshCommandOptBool(cmd, blockdev))
+flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;
if (vshCommandOptStringReq(ctl, cmd, dest, base) 0)
goto cleanup;
ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
@@ -1828,6 +1830,10 @@ static const vshCmdOptDef opts_block_copy[] = {
.type = VSH_OT_BOOL,
.help = N_(use raw destination file)
},
+{.name = blockdev,
+ .type = VSH_OT_BOOL,
+ .help = N_(copy destination is block device instead of regular file)
+},
{.name = wait,
.type = VSH_OT_BOOL,
.help = N_(wait for job to reach mirroring phase)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f07deec..8178868 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -873,7 +873,8 @@ unlimited. The hypervisor can choose whether to reject the
value or
convert it to the maximum value allowed.
=item Bblockcopy Idomain Ipath Idest [Ibandwidth] [I--shallow]
-[I--reuse-external] [I--raw] [I--wait [I--async] [I--verbose]]
+[I--reuse-external] [I--raw] [I--blockdev]
+[I--wait [I--async] [I--verbose]]
[{I--pivot | I--finish}] [I--timeout Bseconds]
Copy a disk backing image chain to Idest. By default, this command
@@ -891,7 +892,9 @@ The format of the destination is determined by the first
match in the
following list: if I--raw is specified, it will be raw; if
I--reuse-external is specified, the existing destination is probed
for a format; and in all other cases, the destination format will
-match the source format.
+match the source format. The destination is treated as a regular
+file unless I--blockdev is used to signal that it is a block
+device.
By default, the copy job runs in the background, and consists of two
phases. Initially, the job must copy all data from the source, and
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] questions on backing chain file, block lun, active commit
On 13/08/14 19:48 -0600, Eric Blake wrote: While investigating active commit, I noticed that our code for probing backing chains currently guesses at type=file vs. type=block solely based on stat results (regular file vs. block device). Is a block device allowed to be used as disk type='block' device='lun' at the same time it has qcow2 format, or does use of device='lun' enforce that the block device is used with raw format? My worry is that if a lun device can be used with qcow2 format, then it can have a non-block backing file, and an active commit would convert disk type='block' device='lun' into disk type='file' device='lun' which is not valid. On a related vein, that means that an active commit currently auto-picks whether the disk will be listed as type='file' or type='block' solely on the stat results. Elsewhere, we allow type='file' even for block devices, where a noticeable difference is how virDomainGetBlockInfo() reports allocation (for type='file', allocation is based on how sparse the file is, but a block device is not sparse; for type='block', allocation is based on asking qemu the maximum used cluster). Should we be auto-converting to type='block' in other cases where we have stat results? For example, I posted a patch today to let the user explicitly request that virsh blockcopy to a block device will use type='block' because the existing code has always used type='file', but if we were doing things automatically, then the user would automatically get type='block' for block device destinations instead of having to request it; but has no way to forcefully list a file even when the destination is a block (at least, not until I implement virDomainBlockCopy() that takes an XML disk description). One drawback of autoconversion is the XML changes - with type='file', the host resource is located at xpath disk/source/@file, but with type='block', it is at xpath disk/source/@dev. If we ever convert a user's type='file' into a block device, but the user isn't expecting the conversion, then they won't be able to find the source file name in the output XML. So on that ground, autoprobing type='block' for active commit is fishy; we really should be honoring the user's input for backingStore rather than probing it ourselves every time. Until that point, and given that I just proposed a patch to turn on type='block' for blockcopy, where type='file' is used in all other cases, should we have the same sort of flag for active commit? Would the following semantics be desirable with respect to disk type conversion? We introduce flag(s) to permit type conversion from file-block (do we need one for the other way too)? For active commit, if one of these flags is not specified, no conversion will be done. If the parent volume type is not the same as the active layer then an error will be trigerred. In other words, the user is responsible for indicating which way a conversion should go if the types are different. -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: exposing backing store allocation in domain xml
On 06/08/14 10:36 -0600, Eric Blake wrote: Adam Litke has been asking if I can expose watermark information from qemu when doing block commit. Qemu still doesn't expose that information when doing 'virsh blockcopy' (QMP drive-mirror), but DOES expose it for regular and active 'virsh blockcommit'. The idea is that when you are writing to more than one file at a time, management needs to know if the file is nearing a watermark for usage that necessitates growing the storage volume before hitting an ENOSPC error. In particular, Adam's use is running qcow2 format on top of block devices, where it is easy to enlarge the block device. The current libvirt API virDomainBlockInfo() can only get watermark information for the active image in a disk chain. It shows three numbers: capacity: the disk size seen by the guest (can be grown via virt-resize) - usually larger than the host block device if the guest has not used the complete disk, but can also be smaller than the host block device due to overhead of qcow2 and the disk is mostly in use allocation: the known usage of the host file/block device, should never be larger than the physical size (other than rounding up to file sector sizing). For sparse files, this number is smaller than total size based by the amount of holes in the file. For block devices with qcow2 format, this number is reported by qemu as the maximum offset in use by the qcow2 file (without regards to whether earlier offsets are holes that could be reused). Compare this to what 'du' would report. physical: the total size of the host file/block device. Compare this to what 'ls' would report. Also, the libvirt API virStorageVolGetXMLDesc reports two of those numbers for a top-level image: capacity and allocation are listed as siblings of target. But it is not present for a backingStore; you have to use the API twice. Now that we have a common virStorageSourcePtr type in the C code, we could do a better job of exposing full information for the entire chain in a single API call. I've got a couple ideas of where we can extend existing APIs (and the extensions do not involve bumping the .so versioning, so it can also be backported, although it gets MUCH harder to backport without virStorageSourcePtr). First, I think the virStorageVolGetXMLDesc should show all three numbers, by adding a physical unit='bytes'.../physical element alongside the existing capacity and allocation elements. Also, I This seems like an obvious improvement especially to make the APIs more symetric. think it might be nice if we could enhance the API to do a full chain recursion (probably requires an explicit flag to turn on) where it shows details on the full backing chain, rather than just partial details on the immediate backing file; in doing that, the backingStore element would gain recursive backingStore (similar to what we recently did in domain XML). In that mode, each layer of backingStore would also report capacity, allocation, and physical. Something like: +1 # virsh vol-dumpxml --pool default f20.snap2 volume type='file' namef20.snap2/name key/var/lib/libvirt/images/f20.snap2/key source /source capacity unit='bytes'12884901888/capacity allocation unit='bytes'2503548928/allocation physical unit='bytes'2503548928/allocation target path/var/lib/libvirt/images/f20.snap2/path format type='qcow2'/ permissions mode0600/mode owner0/owner group0/group labelsystem_u:object_r:virt_image_t:s0/label /permissions timestamps atime1407295598.583411967/atime mtime1403064822.622766566/mtime ctime1404318525.899951254/ctime /timestamps compat1.1/compat features/ /target backingStore path/var/lib/libvirt/images/f20.snap1/path capacity unit='bytes'12884901888/capacity allocation unit='bytes'2503548928/allocation physical unit='bytes'2503548928/allocation format type='qcow2'/ permissions mode0600/mode owner107/owner group107/group labelsystem_u:object_r:virt_content_t:s0/label /permissions timestamps atime1407295598.623411816/atime mtime1402005765.810488875/mtime ctime1404318523.313955796/ctime /timestamps compat1.1/compat features/ backingStore path/var/lib/libvirt/images/f20.base/path capacity unit='bytes'10737418240/capacity allocation unit='bytes'2503548928/allocation physical unit='bytes'10737418240/allocation format type='raw'/ permissions mode0600/mode owner107/owner group107/group labelsystem_u:object_r:virt_content_t:s0/label /permissions timestamps atime1407295598.623411816/atime mtime1402005765.810488875/mtime ctime1404318523.313955796/ctime /timestamps backingStore/ /backingStore /backingStore /volume Also, the current storage volume API is rather hard-coded to assume that backing elements are in the same storage pool, which is not always true. It may be time
Re: [libvirt] [PATCH] blockjob: correctly report active commit for job info
On 05/08/14 08:51 -0600, Eric Blake wrote:
Commit 232a31b munged job info to report 'active commit' instead of
'commit' when generating events, but forgot to also munge the polling
variant of the command.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust type as
needed.
Signed-off-by: Eric Blake ebl...@redhat.com
Reviewed-by: Adam Litke ali...@redhat.com
---
src/qemu/qemu_driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a3de784..57cc913 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15103,6 +15103,9 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
bandwidth, info, mode, async);
qemuDomainObjExitMonitor(driver, vm);
+if (info info-type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT
+disk-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+info-type = disk-mirrorJob;
if (ret 0) {
if (mode == BLOCK_JOB_ABORT disk-mirror)
disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
--
1.9.3
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 18/19] qemu: Add support for networked disks for block commit
On 25/06/14 10:27 -0600, Eric Blake wrote:
On 06/19/2014 07:59 AM, Peter Krempa wrote:
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
Eventually, we'll want to use qemu's node-name functionality, also being
added (but possibly in qemu 2.2 instead of 2.1, depends on how Jeff's
series goes). But for the simpler case of all files being local or all
files being network from the same pool (that is, no mixed-mode chains),
then this does appear to work at getting a decent name into qemu, at
which point qemu can indeed commit to the right target.
---
src/qemu/qemu_driver.c | 39 ++-
1 file changed, 34 insertions(+), 5 deletions(-)
+
+if (flags VIR_DOMAIN_BLOCK_COMMIT_RELATIVE
+topSource != disk-src) {
So you are silently ignoring the flag if topSource is the active layer?
That's okay, but reflect it in the documentation earlier in the series.
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support relative blockpull));
+goto endjob;
+}
+
+if (virStorageFileGetRelativeBackingPath(topSource, baseSource,
+ backingPath) 0)
+goto endjob;
+
+if (!backingPath) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't keep relative backing relationship.));
No '.' at end of the message.
Wait - the earlier patches said that relative names would be preserved
if possible, implying that an absolute name would still be used if a
relative name was not possible. But this errors out if a relative name
was not possible. Which is nicer to the end user, treating the flag as
advisory or mandatory? I'm hoping Adam can answer which he'd prefer, as
one of the first clients of this new feature.
Thanks Eric. If the flag was specified we need it to fail if a
relative backing path is not possible. Otherwise the backing chain
could be rewritten such that the VM can not be started on a different
host in the future. For us, not honoring the flag is a corruption.
For those applications that don't mind (or might handle abs paths
differently than relative ones, they could retry the operation without
the flag. Perhaps we'll want a specific error code for this scenario
to make it easy to handle?
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 19/19] qemu: Add support for networked disks for block pull/block rebase
On 25/06/14 10:34 -0600, Eric Blake wrote:
On 06/19/2014 07:59 AM, Peter Krempa wrote:
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
src/qemu/qemu_driver.c | 45 +
1 file changed, 41 insertions(+), 4 deletions(-)
Same caveats as in 18/19 about not necessarily working in mixed-source
chains (for that, we'd need to use node-names); but as it is definitely
more powerful than what libvirt previously supported, it's still worth
including under the incremental improvement umbrella.
@@ -15040,6 +15042,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
goto cleanup;
}
+if (flags VIR_DOMAIN_BLOCK_REBASE_RELATIVE !base) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only
+ with non-null base ));
Trailing space in the error message. This treats relative name with no
base as a hard error, which is okay but should be documented.
+
+if (!backingPath) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't keep relative backing relationship.));
No trailing '.'. Once again, back to the question of whether it is
nicer for the flag to be advisory (best effort to use relative, but
absolute fallback is okay) or mandatory (fail if the request cannot be
honored).
At this point, I'm leaning towards mandatory (it's easier to relax
mandatory to advisory later than it is to give advisory now and tighten
it up later; and I like to know if my explicit request cannot be
honored). But the documentation needs to match what we choose, and it
would help to have Adam's insight as a client of this flag.
See response to 18... Mandatory is our strict requirement.
--
Adam Litke
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] blockjob: use stable disk string in job event
On 16/06/14 09:52 -0600, Eric Blake wrote: On 06/16/2014 09:45 AM, Daniel P. Berrange wrote: On Sat, Jun 14, 2014 at 07:49:40AM -0600, Eric Blake wrote: If you think backward compatibility of old clients that expected and operated on an absolute name is too important to break, I'm open to suggestions on alternatives how to preserve that. We don't have a flag argument during event registration, or I would have used it. Otherwise, I hope this change is okay as-is. Hmm, I fear this is one of those cases where it seems fine to change the semantics, but is liable to bite us later. The most obvious alternative is to just introduce a new event BLOCK_JOB_EVENT_2 or something less sucky. That still leaves the question of how to generate the old event in the fact of a disk with no local name - we could use empty string '' as the path if the disk lacks one. Or we could simply not emit the old style event. I'd probably go for empty string though. Adam was the one that brought this issue up in the first place. Adam, would you be okay registering for a new event id that guarantees you get a stable name? Yeah, this would be fine for me. Libvirt would have to be able to generate both old and new style events (yuck for me to code up, but such is life of back-compat), but Dan has a valid point that the change is subtle enough that it might bite us if we don't have some way to get both the old and the new style event by explicit user request. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Active commit
On 11/06/14 10:27 -0600, Eric Blake wrote: I still don't have qemu capability detection working reliably, but want to post this series right now so that it can be built into a scratch build containing Peter's and my changes. (Or put another way, I was testing what conflict resolutions are required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some conflicts with Peter's addition of relative backing name; and I think the resolutions were fairly simple). These patches are tested on top of: https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html I may still end up posting a v4 and/or pushing my series before Peter's, once I get capability detection working the way I want. Eric Blake (5): virsh: improve blockcopy UI virsh: expose new active commit controls blockcommit: update error messages related to block jobs blockcommit: track job type in xml blockcommit: turn on active commit I've tested this with the following script and it seems that specifying base and top explicitly does not work. Using --shallow does work for me though. Running the included script yields the following output: $ sudo bash /home/alitke/test/test.sh Formatting 'base.img', fmt=raw size=1073741824 Formatting 'snap1.img', fmt=qcow2 size=1073741824 backing_file='base.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off Formatting 'snap2.img', fmt=qcow2 size=1073741824 backing_file='snap1.img' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off image: /tmp/snap2.img file format: qcow2 virtual size: 1.0G (1073741824 bytes) disk size: 196K cluster_size: 65536 backing file: snap1.img (actual path: /tmp/snap1.img) backing file format: qcow2 Format specific information: compat: 1.1 lazy refcounts: false Domain testvm1 created from /dev/stdin error: invalid argument: could not find image '/tmp/snap2.img' in chain for '/tmp/snap2.img' Domain testvm1 destroyed -- #!/bin/sh rm -f base.img snap1.img snap2.img # base.img - snap1.img - snap2.img qemu-img create -f raw base.img 1G qemu-img create -f qcow2 -b base.img -o backing_fmt=raw snap1.img qemu-img create -f qcow2 -b snap1.img -o backing_fmt=qcow2 snap2.img chcon -t svirt_image_t base.img snap1.img snap2.img chmod 666 base.img snap1.img snap2.img qemu-img info $PWD/snap2.img virsh create /dev/stdin EOF domain type='kvm' nametestvm1/name memory unit='MiB'256/memory vcpu1/vcpu os type arch='x86_64'hvm/type /os devices disk type='file' device='disk' driver name='qemu' type='qcow2'/ source file='$PWD/snap2.img'/ target dev='vda' bus='virtio'/ /disk graphics type='vnc'/ /devices /domain EOF base=$PWD/snap1.img top=$PWD/snap2.img virsh blockcommit testvm1 vda 0 $base $top --wait --verbose --pivot virsh destroy testvm1 -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: disallow bandwidth/mac for bridged/macvtap networks
On 24/01/14 14:18 +0200, Laine Stump wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1057321 pointed out that
we weren't honoring the bandwidth element in libvirt networks using
forward mode='bridge'/. In fact, these networks are just a method of
giving a libvirt network name to an existing Linux host bridge on the
system, and even if it were technically possible for us to set
network-wide bandwidth limits for all the taps on a bridge, it's
probably not a polite thing to do since libvirt is just using a bridge
that was created by someone else for other purposes. So the proper
thing is to just log an error when someone tries to put a bandwidth
element in that type of network.
While looking through the network XML documentation and comparing it
to the networkValidate function, I noticed that we also ignore the
presence of a mac address in the config, even though we do nothing
with it in this case either.
This patch updates networkValidate() (which is called any time a
persistent network is defined, or a transient network created) to log
an error and fail if it finds either a bandwidth or mac element
and the network forward mode is anything except 'route'. 'nat', or
nothing. (Yes, neither of those elements is acceptable for any macvtap
mode, nor for a hostdev network).
NB: This does *not* cause failure to start any existing network that
contains one of those elements, so someone might have erroneously
defined such a network in the past, and that network will continue to
function unmodified. I considered it too disruptive to suddenly break
working configs on the next reboot after a libvirt upgrade.
Reviewed-by: Adam Litke ali...@redhat.com
---
src/network/bridge_driver.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0b43a67..3b9b58d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2407,8 +2407,17 @@ networkValidate(virNetworkDriverStatePtr driver,
virNetworkSetBridgeMacAddr(def);
} else {
/* They are also the only types that currently support setting
- * an IP address for the host-side device (bridge)
+ * a MAC or IP address for the host-side device (bridge), DNS
+ * configuration, or network-wide bandwidth limits.
*/
+if (def-mac_specified) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported mac element in network %s
+ with forward mode='%s'),
+ def-name,
+ virNetworkForwardTypeToString(def-forward.type));
+return -1;
+}
if (virNetworkDefGetIpByIndex(def, AF_UNSPEC, 0)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported ip element in network %s
@@ -2433,6 +2442,14 @@ networkValidate(virNetworkDriverStatePtr driver,
virNetworkForwardTypeToString(def-forward.type));
return -1;
}
+if (def-bandwidth) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported network-wide bandwidth element
+ in network %s with forward mode='%s'),
+ def-name,
+ virNetworkForwardTypeToString(def-forward.type));
+return -1;
+}
}
/* We only support dhcp on one IPv4 address and
--
1.8.5.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 02/15] blockjob: wire up qemu async virDomainBlockJobAbort
On Thu, Apr 05, 2012 at 10:36:48PM -0600, Eric Blake wrote:
From: Adam Litke a...@us.ibm.com
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally
poll
using qemu's query-block-jobs API and will not return until the operation
has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block. Future
patches may refactor things to allow other queries in parallel with this
polling. Unfortunately, there's no good way to tell if qemu will emit the
new event, so this implementation always polls to deal with older qemu.
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Eric Blake ebl...@redhat.com
Tested-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_driver.c | 55 +--
1 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0456b34..455fa30 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11602,7 +11602,7 @@ cleanup:
static int
qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
unsigned long bandwidth, virDomainBlockJobInfoPtr
info,
- int mode)
+ int mode, unsigned int flags)
{
struct qemud_driver *driver = dom-conn-privateData;
virDomainObjPtr vm = NULL;
@@ -11644,6 +11644,45 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char
*path, const char *base,
ret = qemuMonitorBlockJob(priv-mon, device, base, bandwidth, info,
mode);
qemuDomainObjExitMonitorWithDriver(driver, vm);
+/* Qemu provides asynchronous block job cancellation, but without
+ * the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag libvirt guarantees a
+ * synchronous operation. Provide this behavior by waiting here,
+ * so we don't get confused by newly scheduled block jobs.
+ */
+if (ret == 0 mode == BLOCK_JOB_ABORT
+!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+ret = 1;
+while (1) {
+/* Poll every 50ms */
+struct timespec ts = { .tv_sec = 0,
+ .tv_nsec = 50 * 1000 * 1000ull };
+virDomainBlockJobInfo dummy;
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorBlockJob(priv-mon, device, NULL, 0, dummy,
+ BLOCK_JOB_INFO);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+if (ret = 0)
+break;
+
+virDomainObjUnlock(vm);
+qemuDriverUnlock(driver);
+
+nanosleep(ts, NULL);
+
+qemuDriverLock(driver);
+virDomainObjLock(vm);
+
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(domain is not running));
+ret = -1;
+break;
+}
+}
+}
+
endjob:
if (qemuDomainObjEndJob(driver, vm) == 0) {
vm = NULL;
@@ -11661,8 +11700,9 @@ cleanup:
static int
qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int
flags)
{
-virCheckFlags(0, -1);
-return qemuDomainBlockJobImpl(dom, path, NULL, 0, NULL, BLOCK_JOB_ABORT);
+virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC, -1);
+return qemuDomainBlockJobImpl(dom, path, NULL, 0, NULL, BLOCK_JOB_ABORT,
+ flags);
}
static int
@@ -11670,7 +11710,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const
char *path,
virDomainBlockJobInfoPtr info, unsigned int flags)
{
virCheckFlags(0, -1);
-return qemuDomainBlockJobImpl(dom, path, NULL, 0, info, BLOCK_JOB_INFO);
+return qemuDomainBlockJobImpl(dom, path, NULL, 0, info, BLOCK_JOB_INFO,
+ flags);
}
static int
@@ -11679,7 +11720,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const
char *path,
{
virCheckFlags(0, -1);
return qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
- BLOCK_JOB_SPEED);
+ BLOCK_JOB_SPEED, flags);
}
static int
@@ -11690,10 +11731,10 @@ qemuDomainBlockRebase(virDomainPtr dom, const char
*path, const char *base,
virCheckFlags(0, -1);
ret = qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
- BLOCK_JOB_PULL);
+ BLOCK_JOB_PULL, flags);
if (ret == 0 bandwidth != 0)
ret = qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
- BLOCK_JOB_SPEED);
+ BLOCK_JOB_SPEED, flags);
return ret
Re: [libvirt] [PATCHv2 01/15] blockjob: add API for async virDomainBlockJobAbort
On Thu, Apr 05, 2012 at 10:36:47PM -0600, Eric Blake wrote:
From: Adam Litke a...@us.ibm.com
Qemu has changed the semantics of the block_job_cancel API. The original
qed implementation (pretty much only backported to RHEL 6.2 qemu) was
synchronous (ie. upon command completion, the operation was guaranteed to
be completely stopped). With the new semantics going into qemu 1.1 for
qcow2, a block_job_cancel merely requests that the operation be cancelled
and an event is triggered once the cancellation request has been honored.
To adopt the new semantics while preserving compatibility the following
updates are made to the virDomainBlockJob API:
A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED is recognized by
libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event
will be raised whenever it is received from qemu. This event indicates that a
block job has been successfully cancelled. For now, libvirt does not try
to synthesize this event if using an older qemu that did not generate it.
A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC is added to the
virDomainBlockJobAbort API. When enabled, this function will operate
asynchronously (ie, it can return before the job has actually been cancelled).
When the API is used in this mode, it is the responsibility of the caller to
wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
virDomainGetBlockJobInfo API to check the cancellation status; this flag
is an error if it is not known if the hypervisor supports asynchronous cancel.
This patch also exposes the new flag through virsh.
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Eric Blake ebl...@redhat.com
Tested-by: Adam Litke a...@us.ibm.com
---
include/libvirt/libvirt.h.in | 10 +
src/libvirt.c| 10 -
src/qemu/qemu_monitor_json.c | 42 +++--
tools/virsh.c| 46 ++---
tools/virsh.pod |9 +--
5 files changed, 89 insertions(+), 28 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 499dcd4..97ad99d 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1946,6 +1946,15 @@ typedef enum {
#endif
} virDomainBlockJobType;
+/**
+ * virDomainBlockJobAbortFlags:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1 0,
+} virDomainBlockJobAbortFlags;
+
/* An iterator for monitoring block job operations */
typedef unsigned long long virDomainBlockJobCursor;
@@ -3617,6 +3626,7 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
+VIR_DOMAIN_BLOCK_JOB_CANCELED = 2,
#ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_BLOCK_JOB_LAST
diff --git a/src/libvirt.c b/src/libvirt.c
index 16d1fd5..af22232 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17902,7 +17902,7 @@ error:
* virDomainBlockJobAbort:
* @dom: pointer to domain object
* @disk: path to the block device, or device shorthand
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainBlockJobAbortFlags
*
* Cancel the active block job on the given disk.
*
@@ -17913,6 +17913,14 @@ error:
* can be found by calling virDomainGetXMLDesc() and inspecting
* elements within //domain/devices/disk.
*
+ * By default, this function performs a synchronous operation and the caller
+ * may assume that the operation has completed when 0 is returned. However,
+ * BlockJob operations may take a long time to complete, and during this time
+ * further domain interactions may be unresponsive. To avoid this problem,
+ * pass VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC in the @flags argument to enable
+ * asynchronous behavior. Either way, when the job has been cancelled, a
+ * BlockJob event will be emitted, with status
VIR_DOMAIN_BLOCK_JOB_CANCELLED.
+ *
* Returns -1 in case of failure, 0 when successful.
*/
int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8eeb307..9a8da3a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -58,13 +58,14 @@ static void qemuMonitorJSONHandleIOError(qemuMonitorPtr
mon, virJSONValuePtr dat
static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon,
virJSONValuePtr data);
static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon,
virJSONValuePtr data);
static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon,
virJSONValuePtr data);
-static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon,
virJSONValuePtr data
Re: [libvirt] RFC API proposal: virDomainBlockRebase
of a current image (that is, undoing a current snapshot): merge current into intermediate, creating: template - intermediate and backward merge of a non-current image (that is, undoing an earlier snapshot, but by modifying the template rather than the current image): merge intermediate into base, creating: template - current Don't these raise some security concerns about modifying a potentially shared intermediate image? Backward merge of the current image seems like something easy to fit into my proposed API (add a new flag, maybe called VIR_DOMAIN_BLOCK_REBASE_BACKWARD). Manipulations of anything that does not involve the current image seems tougher, assuming qemu ever even reaches the point where it exposes those operations on live volumes - the user has to specify not one, but two backing file names. But even that could possibly be fit into my API, by adding a flag that states that the const char *backing argument is treated as an XML snippet describing the full details of the merge, with the XML listing which image is being merged to which destination, rather than as just the name of the backing file becoming the new base of the current image. Perhaps something like: virDomainBlockRebase(dom, block, rebase\n source/path/to/intermediate/source\n dest/path/to/template/dest\n /rebase, VIR_DOMAIN_BLOCK_REBASE_XML|VIR_DOMAIN_BLOCK_REBASE_BACKWARD) as a specification to take the contents of intermediate, merge those backwards into template, and as well as adjusting the rest of the backing file chain so that whatever used to be backed by intermediate is now backed by template. Or, if qemu ever gives us the ability to merge non-current images, we may decide at that time that it is worth a new API to expose those new complexities. This is all starting to scare me so I will defer to the storage pros :) Another thing I have been thinking about is virDomainSnapshotDelete. The above conversation talks about merging of a single disk, but a live disk snapshot operation can create backing file chains for multiple disks at once, all tracked by a snapshot. Additionally, the current code allows a snapshot delete of internal snapshots, but refuses to do anything useful with an external snapshot, because there is currently no way to specify if the snapshot is removed by merging the base into the new current, or by undoing the current and merging it backwards into the base. Alas, virDomainSnapshotDelete doesn't take any arguments for how to handle the situation, and use of a flag to make the decision would limit all disks to be handled in the same manner. So what I'm thinking is that when a snapshot is created (or redefined, using redefinition as the vehicle to add in the new XML), that the snapshot XML itself can record the preferred direction for undoing the snapshot; for example: domainsnapshot disks disk name='/path/to/old_vda' source file='/path/to/new_vda'/ on_delete merge='forward'/ /disk disk name='/path/to/old_vdb' source file='/path/to/new_vdb'/ on_delete merge='backward'/ /disk disks /domainsnapshot then when virDomainSnapshotDelete is called on that snapshot, old_vda would be forward merged into new_vda, while new_vdb would be backward merged into old_vdb. Again, that's food for thought for post-0.9.10, and shouldn't get in the way of adding virDomainBlockRebase() now. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC API proposal: virDomainBlockRebase
On Tue, Jan 31, 2012 at 03:00:40PM -0700, Eric Blake wrote: On 01/31/2012 01:53 PM, Adam Litke wrote: On Tue, Jan 31, 2012 at 09:28:51AM -0700, Eric Blake wrote: Right now, the existing virDomainBlockPull API has a tough limitation - it is an all-or-none approach. In all my examples below, I'm starting from the following relationship, where '-' means 'is a backing file of': template - intermediate - current Meanwhile, qemu is adding support for a partial block pull operation, still on the current image as the merge destination, but where you can now specify an optional argument to limit the pull to just the intermediate files and altering the current image to be backed by an ancestor file, as in: merge intermediate into current, creating: template - current For 0.9.10, I'd like to add the following API: /** * virDomainBlockRebase: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand * @base: new base image, or NULL for entire block pull * @bandwidth: (optional) specify copy bandwidth limit in Mbps * @flags: extra flags; not used yet, so callers should always pass 0 What is the format of the @base arg? My first thought would be a path, but what if the desired image file is not directly known to libvirt? Libvirt already has to know the absolute paths of all disks in the backing file chain, in order to properly SELinux label them prior to invoking qemu. So I'm envisioning the absolute path of the backing file in the chain that will be preserved. That is, with: Ok. That is clear. Thanks. touch 10M /path/to/template qemu-img create -f qcow2 \ -o backing_file /path/to/template /path/to/intermediate qemu-img create -f qcow2 \ -o backing_file /path/to/intermediate /path/to/current followed by virDomainBlockRebase(dom, vda, /path/to/template, 0) would result in /path/to/current referring to /path/to/template as its primary backing file. I also had an idea down below where, with the addition of a new flags value, base could refer to a well-formed XML block rather than a single file name, such that we could then populate that XML block with more complex instructions; but I'm not proposing doing that extension in the 0.9.10 timeframe, so much as trying to argue that this API is extensible and we won't need yet a third API for block pull if qemu ever allows more complex merging scenarios. Given that Adam has a pending patch to support a VIR_DOMAIN_BLOCK_PULL_ASYNC flag, this same flag would have to be supported in virDomainBlockRebase. That patch only applies to virDomainBlockJobCancel(). The blockJob initiators (virDomainBlockPull and this new one) already use an async mode of operation because the call simply starts the block job. Ah, right. I'm getting slightly confused with all the patches that still need review :) virDomainBlockPull has always been asynchronous, so no flag is needed there or in this new API. and backward merge of a non-current image (that is, undoing an earlier snapshot, but by modifying the template rather than the current image): merge intermediate into base, creating: template - current Don't these raise some security concerns about modifying a potentially shared intermediate image? Yes, the management app has to be careful to not remove backing files that are in use in other chains. But we already have a lock manager setup, so of course, part of the libvirt work is integrating this work so that no image is ever reverted if the lock manager says a file is in use; libvirt can also check all registered storage pools and prevent a rebase if the storage pool tracks files that serve as base images to more than one other images, and prevent modifying base images in those cases (there's probably still a lot of work to be done in libvirt to make it bulletproof, but that's okay, since again, my ideas about reverse merges are post-0.9.10 and would also require more work from qemu). At any rate, you responded favorably to the first half of my email (the proposal for what to implement in 0.9.10), even if you got scared by my musings about possible future extensions at later releases. I'll take that as a good sign that I have a) come up with a good API worth adding now, and b) divided things appropriately into what is reasonable to do now vs. what is complex enough to be worth delaying until we have more experience with the use cases and ramifications of adding the complexity. Yes, exactly. Seems like a good plan. I am happy to see that the blockJob API family will be extended as we initially intended. -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] add a qemu-specific event register API, to passthough the new events come from qemu
On Wed, Jan 25, 2012 at 02:16:14PM -0700, Eric Blake wrote: On 12/16/2011 09:58 AM, shao...@linux.vnet.ibm.com wrote: From: ShaoHe Feng shao...@linux.vnet.ibm.com Basically, this feature can go along with qemu monitor passthrough. That way, if we use new commands in the monitor that generate new events, we want some way to receive those new events too. Signed-off-by: ShaoHe Feng shao...@linux.vnet.ibm.com --- include/libvirt/libvirt-qemu.h | 27 include/libvirt/libvirt.h.in |2 +- src/conf/domain_event.c| 293 ++-- src/conf/domain_event.h| 50 ++- src/driver.h | 14 ++ src/libvirt-qemu.c | 189 ++ src/libvirt_private.syms |6 + src/libvirt_qemu.syms |5 + 8 files changed, 571 insertions(+), 15 deletions(-) Where do we stand on this series? It seems like something worth including in 0.9.10; can we get it rebased to the latest libvirt so it can get a good review? Thanks for asking about this... Shaohe is on vacation for two weeks so I will do my best to answer in his absence. I believe the process of moving the event code into libvirt_qemu.so had increased code size, complexity, and duplication. He is still looking at improving the series and will be posting another RFC soon after he returns from vacation. Therefore, we'll have to punt this one to the next release. -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
On Thu, Jan 19, 2012 at 05:39:43PM -0700, Eric Blake wrote: On 01/13/2012 01:51 PM, Adam Litke wrote: Qemu has changed the semantics of the block_job_cancel API. Originally, the operation was synchronous (ie. upon command completion, the operation was guaranteed to be completely stopped). With the new semantics, a block_job_cancel merely requests that the operation be cancelled and an event is triggered once the cancellation request has been honored. How can we tell the two implementations of qemu apart? That is, how is libvirt supposed to know whether it is calling the old (blocking) variant, or the new async variant, in order to know whether it should wait for an event? On the other hand, I think we can make things work: call cancel, then call job status if the job is complete, then we are done, whether or not we have old or new command semantics (and if we have new command semantics, be prepared to ignore an event from qemu) I don't really like ignoring domain events based on one specific API invocation -- see comment farther below. if the job is not complete, the we have the new semantics, and we know to expect the event Pretty clever, but there is a very small race window where the cancel completed and a new job has started immediately. In that case, we could assume we have new command semantics when we actually have an old qemu. coupled with: if the user passed 0 for the flag (they want blocking), then make the command wait for completion (either by job-status query or by getting the event), and no event is passed to the user if the user passed ASYNC for the flag, then they want an event; if we detect the async variant of the command, then we hook up our qemu event handler to turn around and issue a user event. Conversely, if job-query says the command is complete, then we generate our own event without waiting for a qemu event. That means that we _don't_ blindly pass the qemu event on to the user; rather, on reception of a qemu event, we determine if we have an incomplete cancel in flight, and only then do we pass it on to the user; otherwise, we assume that the qemu event came after we already saw the job-info query saying things were complete, and therefore, we had already generated the user's event and can ignore the qemu event. It would help things if the new qemu command were spelled with a different name than the old blocking version. To adopt the new semantics while preserving compatibility I propose the following updates to the virDomainBlockJob API: A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event will be raised whenever it is received from qemu. This event indicates that a block job has been successfully cancelled. I'm not sure it should be raised unconditionally on receipt of a qemu event, but only in the case of a virDomainBlockJobAbort(ASYNC) where we detect that cancel is in flight when we return control to the user. What happens if an impatient user calls cancel several times on multiple devices, sometimes using ASYNC and sometimes not). Clearly then, we should only raise events for the devices that the user passed the ASYNC flag. We are going to need a flag per disk that specifies whether block_job_cancel events are user-deliverable. This is all getting pretty complicated. Can we take a step back and look at how we can simplify this? I just thought of another reason why I would prefer not to mask out events based on the initial API call. Imagine a libvirtd where multiple users/applications are connected. User A is a disk manager that monitors BlockJob operations (for logging and VM placement decision making). User B is an administrator. If we mask the cancel event depending on User B's invocation of the API, then User A will not be guaranteed to see a consistent view of what is happening on the system (ie. it may not receive some events it was expecting). I am not sure if this is practical (or desireable), but here is an idea: - We always deliver any events that may arise - For the case where an event is requested but qemu is too old to issue one: * Create a new timer with a callback that calls query-block-jobs to check for active jobs. If no active jobs are found, issue the event. Otherwise, reset the timer. (We would need to handle the case where a new job is created before the timer fires. This isn't that difficult). A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the virDomainBlockJobAbort API. When enabled, this function will operate asynchronously (ie, it can return before the job has actually been cancelled). When the API is used in this mode, it is the responsibility of the caller to wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the virDomainGetBlockJobInfo API to check
Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
On Wed, Jan 18, 2012 at 12:59:29PM -0500, Federico Simoncelli wrote: - Original Message - From: Adam Litke a...@us.ibm.com To: Dave Allan dal...@redhat.com Cc: libvir-list@redhat.com Sent: Friday, January 13, 2012 9:51:23 PM Subject: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort Qemu has changed the semantics of the block_job_cancel API. Originally, the operation was synchronous (ie. upon command completion, the operation was guaranteed to be completely stopped). With the new semantics, a block_job_cancel merely requests that the operation be cancelled and an event is triggered once the cancellation request has been honored. To adopt the new semantics while preserving compatibility I propose the following updates to the virDomainBlockJob API: A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event will be raised whenever it is received from qemu. This event indicates that a block job has been successfully cancelled. [...] Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll using qemu's query-block-jobs API and will not return until the operation has been completed. Why do you raise the event VIR_DOMAIN_BLOCK_JOB_CANCELLED also in the case where libvirt takes care of it internally? Is there a specific use case for this? We just raise the events we receive from qemu and qemu will always generate these. IMO, there is no harm in always raising the events. If a user doesn't care about them, then they don't have to register for notifications. API users are advised that this operation is unbounded and further interaction with the domain during this period may block. Do you have an example of what operation may block? Is this the common behavior for all the other async tasks that libvirt is managing internally? Any operation that needs to issue a monitor command would block. This is a large list. That being said, the existing implementation already has this potential issue with the ASYNC flag being the workaround. I cannot answer definitively as to whether all libvirt async tasks can block. I suspect the answer depends on whether the operation relies on a monitor command that can block in qemu. As far as I know the only other command like this (coincidentally it is being improved in qemu by Luiz) is virDomainMemoryStats(). -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] events: Return the correct number of registered events
Commit d09f6ba5feb655925175dc80122ca2a1e14db2b9 introduced a regression in event
registration. virDomainEventCallbackListAddID() will only return a positive
integer if the type of event being registered is VIR_DOMAIN_EVENT_ID_LIFECYCLE.
For other event types, 0 is always returned on success. This has the
unfortunate side effect of not enabling remote event callbacks because
remoteDomainEventRegisterAny() uses the return value from the local call to
determine if an event callback needs to be registered on the remote end.
Make sure virDomainEventCallbackListAddID() returns the callback count for the
eventID being registered.
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/conf/domain_event.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 3fd3ed2..0431697 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -434,7 +434,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
event-callbackID = cbList-nextID++;
for (i = 0 ; i cbList-count ; i++) {
-if (cbList-callbacks[i]-eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE
+if (cbList-callbacks[i]-eventID == eventID
cbList-callbacks[i]-conn == conn
!cbList-callbacks[i]-deleted)
ret++;
--
1.7.5.rc1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
Qemu has changed the semantics of the block_job_cancel API. Originally, the
operation was synchronous (ie. upon command completion, the operation was
guaranteed to be completely stopped). With the new semantics, a
block_job_cancel merely requests that the operation be cancelled and an event
is triggered once the cancellation request has been honored.
To adopt the new semantics while preserving compatibility I propose the
following updates to the virDomainBlockJob API:
A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by
libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event
will be raised whenever it is received from qemu. This event indicates that a
block job has been successfully cancelled.
A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the
virDomainBlockJobAbort API. When enabled, this function will operate
asynchronously (ie, it can return before the job has actually been cancelled).
When the API is used in this mode, it is the responsibility of the caller to
wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
virDomainGetBlockJobInfo API to check the cancellation status.
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll
using qemu's query-block-jobs API and will not return until the operation has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block.
This patch implements the new event type, the API flag, and the polling. The
main outstanding issue is whether we should bound the amount of time we will
wait for cancellation and return an error.
Comments on this proposal?
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Cc: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 10 ++
src/libvirt.c|9 -
src/qemu/qemu_driver.c | 24 +---
src/qemu/qemu_monitor.c | 24
src/qemu/qemu_monitor.h |2 ++
src/qemu/qemu_monitor_json.c | 36 +---
6 files changed, 90 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e436f3c..fc7f028 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1776,6 +1776,15 @@ typedef enum {
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
} virDomainBlockJobType;
+/**
+ * virDomainBlockJobAbortFlags:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1,
+} virDomainBlockJobAbortFlags;
+
/* An iterator for monitoring block job operations */
typedef unsigned long long virDomainBlockJobCursor;
@@ -3293,6 +3302,7 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
+VIR_DOMAIN_BLOCK_JOB_CANCELLED = 2,
} virConnectDomainEventBlockJobStatus;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index a540424..0e886f5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17299,7 +17299,7 @@ error:
* virDomainBlockJobAbort:
* @dom: pointer to domain object
* @disk: path to the block device, or device shorthand
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise or of virDomainBlockJobAbortFlags
*
* Cancel the active block job on the given disk.
*
@@ -17310,6 +17310,13 @@ error:
* can be found by calling virDomainGetXMLDesc() and inspecting
* elements within //domain/devices/disk.
*
+ * By default, this function performs a synchronous operation and the caller
+ * may assume that the operation has completed when 0 is returned. However,
+ * BlockJob operations may take a long time to complete, and during this time
+ * further domain interactions may be unresponsive. To avoid this problem,
pass
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC in the flags argument to enable
asynchronous
+ * behavior. When the job has been cancelled, a BlockJob event will be
emitted.
+ *
* Returns -1 in case of failure, 0 when successful.
*/
int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 712f1fc..d971a5f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11379,7 +11379,7 @@ cleanup:
static int
qemuDomainBlockJobImpl(virDomainPtr dom, const char *path,
unsigned long bandwidth, virDomainBlockJobInfoPtr info,
- int mode)
+ int flags, int mode)
{
struct qemud_driver *driver = dom-conn-privateData;
virDomainObjPtr vm = NULL;
@@ -11420,6 +11420,15 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char
*path,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
priv = vm-privateData;
ret
Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
On Fri, Jan 13, 2012 at 04:48:30PM -0500, Dave Allan wrote:
On Fri, Jan 13, 2012 at 02:51:23PM -0600, Adam Litke wrote:
Qemu has changed the semantics of the block_job_cancel API. Originally,
the
operation was synchronous (ie. upon command completion, the operation was
guaranteed to be completely stopped). With the new semantics, a
block_job_cancel merely requests that the operation be cancelled and an
event
is triggered once the cancellation request has been honored.
To adopt the new semantics while preserving compatibility I propose the
following updates to the virDomainBlockJob API:
A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be
recognized by
libvirt. Regardless of the flags used with virDomainBlockJobAbort, this
event
will be raised whenever it is received from qemu. This event indicates
that a
block job has been successfully cancelled.
A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the
virDomainBlockJobAbort API. When enabled, this function will operate
asynchronously (ie, it can return before the job has actually been
cancelled).
When the API is used in this mode, it is the responsibility of the caller to
wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
virDomainGetBlockJobInfo API to check the cancellation status.
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally
poll
using qemu's query-block-jobs API and will not return until the operation
has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block.
This patch implements the new event type, the API flag, and the polling.
The
main outstanding issue is whether we should bound the amount of time we will
wait for cancellation and return an error.
Comments on this proposal?
Hi Adam,
Thanks for the patch. That approach seems reasonable. Re: bounding
the amount of time we wait for cancellation, if the time isn't
bounded, what happens if the cancellation never happens? IMO the most
important thing is to make sure that the caller has a way to return to
a normally functioning state even if the virDomainBlockJobAbort call
is unable to cancel the job.
Yes, agreed. But this brings up the familiar problem with timeouts: selecting
the right amount of time to wait. I don't have a good answer here, but it's not
really all that bad if we bail out too early. In that case we can just return
VIR_ERR_OPERATION_TIMEOUT to the caller and they can retry the operation again.
Unfortunately, these semantics were not present in the initial API. Is adding a
new error condition (timeout) to an existing API allowed?
I'll defer to the other folks on the code, since I am at this point
likely to do more harm than good. :)
Dave
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Cc: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 10 ++
src/libvirt.c|9 -
src/qemu/qemu_driver.c | 24 +---
src/qemu/qemu_monitor.c | 24
src/qemu/qemu_monitor.h |2 ++
src/qemu/qemu_monitor_json.c | 36 +---
6 files changed, 90 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e436f3c..fc7f028 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1776,6 +1776,15 @@ typedef enum {
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
} virDomainBlockJobType;
+/**
+ * virDomainBlockJobAbortFlags:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for
completion
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1,
+} virDomainBlockJobAbortFlags;
+
/* An iterator for monitoring block job operations */
typedef unsigned long long virDomainBlockJobCursor;
@@ -3293,6 +3302,7 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
+VIR_DOMAIN_BLOCK_JOB_CANCELLED = 2,
} virConnectDomainEventBlockJobStatus;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index a540424..0e886f5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17299,7 +17299,7 @@ error:
* virDomainBlockJobAbort:
* @dom: pointer to domain object
* @disk: path to the block device, or device shorthand
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise or of virDomainBlockJobAbortFlags
*
* Cancel the active block job on the given disk.
*
@@ -17310,6 +17310,13 @@ error:
* can be found by calling virDomainGetXMLDesc() and inspecting
* elements within //domain/devices/disk
[libvirt] blockJob events broken
Hi Dan, I noticed that since d09f6ba5feb655925175dc80122ca2a1e14db2b9, blockJob events are not being delivered to registered clients. I did verify that VM lifecycle events are still working fine though. I am confused why your patch has only affected my blockJob events. Do you have any ideas? -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] add a qemu-specific event register API, to passthough the new events come from qemu
wishes to keep the domain object after the callback
returns,
+ * it shall take a reference to it, by calling virDomainRef.
+ * The reference can be released once the object is no longer required
+ * by calling virDomainFree.
+ *
+ * The return value from this method is a positive integer identifier
+ * for the callback. To unregister a callback, this callback ID should
+ * be passed to the virConnectDomainQemuEventDeregister method
+ *
+ * Returns a callback identifier on success, -1 on failure
+ */
+int
+virConnectDomainQemuEventRegister(virConnectPtr conn,
+ virDomainPtr dom, /* option to filter */
+ const char *eventName, /* JSON event name
*/
+ virConnectDomainQemuEventCallback cb,
+ void *opaque,
+ virFreeCallback freecb)
+{
+VIR_DOMAIN_DEBUG(dom, conn=%p, eventName=%s, cb=%p, opaque=%p,
freecb=%p,
+ conn, eventName, cb, opaque, freecb);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+if (dom != NULL
+!(VIR_IS_CONNECTED_DOMAIN(dom) dom-conn == conn)) {
+virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(conn);
+return -1;
+}
+if (eventName == NULL || cb == NULL) {
+virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+if ((conn-driver) (conn-driver-qemuDomainQemuEventRegister)) {
+int ret;
+ret = conn-driver-qemuDomainQemuEventRegister(conn, dom,
eventName, cb, opaque, freecb);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+error:
+virDispatchError(conn);
+return -1;
+}
+
+/**
+ * virConnectDomainQemuEventDeregister:
+ * @conn: pointer to the connection
+ * @callbackID: the callback identifier
+ *
+ * Removes an event callback. The callbackID parameter should be the
+ * vaule obtained from a previous virConnectDomainQemuEventDeregister method.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virConnectDomainQemuEventDeregister(virConnectPtr conn,
+int callbackID)
+{
+
+VIR_DEBUG(conn=%p, callbackID=%d, conn, callbackID);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+if (callbackID 0) {
+virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+if ((conn-driver) (conn-driver-qemuDomainQemuEventDeregister)) {
+int ret;
+ret = conn-driver-qemuDomainQemuEventDeregister(conn, callbackID);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+error:
+virDispatchError(conn);
+return -1;
+}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 48ffdf2..75e544a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -471,11 +471,16 @@ virDomainWatchdogModelTypeToString;
# domain_event.h
virDomainEventBlockJobNewFromObj;
virDomainEventBlockJobNewFromDom;
+virDomainEventUnknownNewFromObj;
+virDomainEventunknownNewFromDom;
virDomainEventCallbackListAdd;
virDomainEventCallbackListAddID;
+virDomainEventCallbackListAddName;
virDomainEventCallbackListCount;
virDomainEventCallbackListCountID;
+virDomainEventCallbackListCountName;
virDomainEventCallbackListEventID;
+virDomainEventCallbackListEventName;
virDomainEventCallbackListFree;
virDomainEventCallbackListMarkDelete;
virDomainEventCallbackListMarkDeleteID;
@@ -512,6 +517,7 @@ virDomainEventRebootNewFromDom;
virDomainEventRebootNewFromObj;
virDomainEventStateDeregister;
virDomainEventStateDeregisterAny;
+virDomainQemuEventStateDeregister;
virDomainEventStateFlush;
virDomainEventStateFree;
virDomainEventStateNew;
diff --git a/src/libvirt_qemu.syms b/src/libvirt_qemu.syms
index 8447730..a17e387 100644
--- a/src/libvirt_qemu.syms
+++ b/src/libvirt_qemu.syms
@@ -19,3 +19,8 @@ LIBVIRT_QEMU_0.9.4 {
global:
virDomainQemuAttach;
} LIBVIRT_QEMU_0.8.3;
+LIBVIRT_QEMU_0.9.9 {
+global:
+virConnectDomainQemuEventRegister;
+virConnectDomainQemuEventDeregister;
+} LIBVIRT_QEMU_0.9.4;
--
1.7.5.4
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] BlockJob: Support sync/async virDomainBlockJobAbort
Qemu has changed the semantics of the block_job_cancel API. Originally, the
operation was synchronous (ie. upon command completion, the operation was
guaranteed to be completely stopped). With the new semantics, a
block_job_cancel merely requests that the operation be cancelled and an event
is triggered once the cancellation request has been honored.
To adopt the new semantics while preserving compatibility I propose the
following updates to the virDomainBlockJob API:
A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by
libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event
will be raised whenever it is received from qemu. This event indicates that a
block job has been successfully cancelled.
A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the
virDomainBlockJobAbort API. When enabled, this function will operate
asynchronously (ie, it can return before the job has actually been cancelled).
When the API is used in this mode, it is the responsibility of the caller to
wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
virDomainGetBlockJobInfo API to check the cancellation status.
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll
using qemu's query-block-jobs API and will not return until the operation has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block.
This patch implements the new event type, the API flag, and the polling. The
main outstanding issue is whether we should bound the amount of time we will
wait for cancellation and return an error.
Comments on this proposal?
Signed-off-by: Adam Litke a...@us.ibm.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Cc: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 10 ++
src/libvirt.c|9 -
src/qemu/qemu_driver.c | 24 +---
src/qemu/qemu_monitor.c | 24
src/qemu/qemu_monitor.h |2 ++
src/qemu/qemu_monitor_json.c | 36 +---
6 files changed, 90 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2480add..08fc1de 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1677,6 +1677,15 @@ typedef enum {
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
} virDomainBlockJobType;
+/**
+ * virDomainBlockJobAbortFlags:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1,
+} virDomainBlockJobAbortFlags;
+
/* An iterator for monitoring block job operations */
typedef unsigned long long virDomainBlockJobCursor;
@@ -3188,6 +3197,7 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
+VIR_DOMAIN_BLOCK_JOB_CANCELLED = 2,
} virConnectDomainEventBlockJobStatus;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index 68074e7..103e246 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17016,7 +17016,7 @@ error:
* virDomainBlockJobAbort:
* @dom: pointer to domain object
* @disk: path to the block device, or device shorthand
- * @flags: currently unused, for future extension
+ * @flags: bitwise or of virDomainBlockJobAbortFlags
*
* Cancel the active block job on the given disk.
*
@@ -17027,6 +17027,13 @@ error:
* can be found by calling virDomainGetXMLDesc() and inspecting
* elements within //domain/devices/disk.
*
+ * By default, this function performs a synchronous operation and the caller
+ * may assume that the operation has completed when 0 is returned. However,
+ * BlockJob operations may take a long time to complete, and during this time
+ * further domain interactions may be unresponsive. To avoid this problem,
pass
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC in the flags argument to enable
asynchronous
+ * behavior. When the job has been cancelled, a BlockJob event will be
emitted.
+ *
* Returns -1 in case of failure, 0 when successful.
*/
int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e5ed9a..18c41d5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10947,7 +10947,7 @@ cleanup:
static int
qemuDomainBlockJobImpl(virDomainPtr dom, const char *path,
unsigned long bandwidth, virDomainBlockJobInfoPtr info,
- int mode)
+ int flags, int mode)
{
struct qemud_driver *driver = dom-conn-privateData;
virDomainObjPtr vm = NULL;
@@ -10988,6 +10988,15 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char
*path,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
priv = vm-privateData;
ret = qemuMonitorBlockJob
Re: [libvirt] [Qemu-devel] virDomainBlockJobAbort and block_job_cancel
On Thu, Nov 24, 2011 at 09:21:42AM +, Stefan Hajnoczi wrote: On Thu, Nov 24, 2011 at 5:31 AM, Daniel Veillard veill...@redhat.com wrote: On Wed, Nov 23, 2011 at 09:04:50AM -0700, Eric Blake wrote: On 11/23/2011 07:48 AM, Stefan Hajnoczi wrote: This means that virDomainBlockJobAbort() returns to the client without a guarantee that the job has completed. If the client enumerates jobs it may still see a job that has not finished cancelling. The client must register a handler for the BLOCK_JOB_CANCELLED event if it wants to know when the job really goes away. The BLOCK_JOB_CANCELLED event has the same fields as the BLOCK_JOB_COMPLETED event, except it lacks the optional error message field. The impact on clients is that they need to add a BLOCK_JOB_CANCELLED handler if they really want to wait. Most clients today (not many exist) will be fine without waiting for cancellation. Any objections or thoughts on this? virDomainBlockJobAbort() thankfully has an 'unsigned int flags' argument. For backwards-compatibility, I suggest we use it: calling virDomainBlockJobAbort(,0) maintains old blocking behavior, and we document that blocking until things abort may render the rest of interactions with the domain unresponsive. The new virDomainBlockJobAbort(,VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC) would then implement your new proposed semantics of returning immediately once the cancellation has been requested, even if it hasn't been acted on yet. Maybe you could convince me to swap the flags: have 0 change semantics to non-blocking, and a new flag to request blocking, where callers that care have to try the flag, and if the flag is unsupported then they know they are talking to older libvirtd where the behavior is blocking by default, but that's a bit riskier. Agreed, I would rather not change the current call semantic, but an ASYNC flag would be a really good addition. We can document the risk of not using it in the function description and suggest new applications use ASYNC flag. Yep, that's a nice suggestion and solves the problem. I am almost ready to post the code that makes the above change, but before I do, I need to ask a question about implementing synchronous behavior. Stefan's qemu tree has a block_job_cancel command that always acts asynchronously. In order to provide the synchronous behavior in libvirt (when flags is 0), I need to wait for the block job to go away. I see two options: 1) Use the event: To implement this I would create an internal event callback function and register it to receive the block job events. When the cancel event comes in, I can exit the qemu job context. One problem I see with this is that events are only available when using the json monitor mode. 2) Poll the qemu monitor To do it this way, I would write a function that repeatedly calls virDomainGetBlockJobInfo() against the disk in question. Once the job disappears from the list I can return with confidence that the job is gone. This is obviously sub-optimal because I need to poll and sleep. 3) Ask Stefan to provide a synchronous mode for the qemu monitor command This one is the nicest from a libvirt perspective, but I doubt qemu wants to add such an interface given that it basically has broken semantics as far as qemu is concerned. If this is all too nasty, we could probably just change the behavior of blockJobAbort and make it always synchronous with a 'cancelled' event. Thoughts? -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- 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 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). -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
;
+}
+
+/**
+ * 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 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.
I think this might be a mistake from copying the comment from somewhere else.
Should it read: Returns -1 if an error occurred, 0 otherwise.?
+ */
+
+int virDomainGetBlockIoTune(virDomainPtr dom,
+const char *disk,
+virDomainBlockIoTuneInfoPtr 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 (!disk) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
Should you also check that reply is not NULL?
+
+if (conn-driver-domainGetBlockIoTune) {
+int ret;
+ret = conn-driver-domainGetBlockIoTune(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 bcefb10..4808891 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -496,6 +496,8 @@ LIBVIRT_0.9.7 {
virDomainSnapshotGetParent;
virDomainSnapshotListChildrenNames;
virDomainSnapshotNumChildren;
+virDomainSetBlockIoTune;
+virDomainGetBlockIoTune;
} LIBVIRT_0.9.5;
# define new API here using predicted next version number
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
iops;
+uint64_t iops_rd;
+uint64_t iops_wr;
+u_int flags;
+};
+struct remote_domain_get_block_io_throttle_ret {
+uint64_t bps;
+uint64_t bps_rd;
+uint64_t bps_wr;
+uint64_t iops;
+uint64_t iops_rd;
+uint64_t iops_wr;
+u_int found;
+};
struct remote_num_of_networks_ret {
intnum;
};
@@ -2005,4 +2036,6 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248,
REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249,
+REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250,
+REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251,
};
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
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
)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s,
+_(info block not supported by this qemu));
+ret = -1;
+goto cleanup;
+}
Please use the function qemuMonitorTextCommandNotFound() instead of open-coding
the above logic here.
+ret = qemuMonitorTextParseBlockIoThrottle(result, device, reply);
+
+cleanup:
+VIR_FREE(cmd);
+VIR_FREE(result);
+return ret;
+}
+
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index f32fce0..1c47d39 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -248,4 +248,14 @@ int qemuMonitorTextOpenGraphics(qemuMonitorPtr mon,
const char *fdname,
bool skipauth);
+int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon,
+ const char *device,
+ virDomainBlockIoTuneInfoPtr info,
+ unsigned int flags);
+
+int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon,
+ const char *device,
+ virDomainBlockIoTuneInfoPtr reply,
+ unsigned int flags);
+
#endif /* QEMU_MONITOR_TEXT_H */
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] Support block I/O throtte in XML
,
nodes[i],
bootMap,
-flags);
+flags,
+ctxt);
if (!disk)
goto error;
@@ -9511,6 +9569,43 @@ virDomainDiskDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, target dev='%s' bus='%s'/\n,
def-dst, bus);
+/*disk I/O throttling*/
+if (def-blkdeviotune.mark) {
+virBufferAddLit(buf, iotune\n);
+if (def-blkdeviotune.total_bytes_sec) {
+virBufferAsprintf(buf,
total_bytes_sec%llu/total_bytes_sec\n,
+ def-blkdeviotune.total_bytes_sec);
+}
+
+if (def-blkdeviotune.read_bytes_sec) {
+virBufferAsprintf(buf,
read_bytes_sec%llu/read_bytes_sec\n,
+ def-blkdeviotune.read_bytes_sec);
+
+}
+
+if (def-blkdeviotune.write_bytes_sec) {
+virBufferAsprintf(buf,
write_bytes_sec%llu/write_bytes_sec\n,
+ def-blkdeviotune.write_bytes_sec);
+}
+
+if (def-blkdeviotune.total_iops_sec) {
+virBufferAsprintf(buf,
total_iops_sec%llu/total_iops_sec\n,
+ def-blkdeviotune.total_iops_sec);
+}
+
+if (def-blkdeviotune.read_iops_sec) {
+virBufferAsprintf(buf,
read_iops_sec%llu/read_iops_sec,
+ def-blkdeviotune.read_iops_sec);
+}
+
+if (def-blkdeviotune.write_iops_sec) {
+virBufferAsprintf(buf,
write_iops_sec%llu/write_iops_sec,
+ def-blkdeviotune.write_iops_sec);
+}
+
+virBufferAddLit(buf, /iotune\n);
+}
+
if (def-bootIndex)
virBufferAsprintf(buf, boot order='%d'/\n, def-bootIndex);
if (def-readonly)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a3cb834..d95e239 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -333,6 +333,18 @@ struct _virDomainDiskDef {
} auth;
char *driverName;
char *driverType;
+
+/*disk I/O throttling*/
+struct {
+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;
+unsigned int mark;
+} blkdeviotune;
+
char *serial;
int cachemode;
int error_policy; /* enum virDomainDiskErrorPolicy */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 11ebb69..3a8575b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1715,6 +1715,39 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
}
}
+/*block I/O throttling*/
+if (disk-blkdeviotune.mark) {
+if (disk-blkdeviotune.total_bytes_sec) {
+virBufferAsprintf(opt, ,bps=%llu,
+ disk-blkdeviotune.total_bytes_sec);
+}
+
+if (disk-blkdeviotune.read_bytes_sec) {
+virBufferAsprintf(opt, ,bps_rd=%llu,
+ disk-blkdeviotune.read_bytes_sec);
+}
+
+if (disk-blkdeviotune.write_bytes_sec) {
+virBufferAsprintf(opt, ,bps_wr=%llu,
+ disk-blkdeviotune.write_bytes_sec);
+}
+
+if (disk-blkdeviotune.total_iops_sec) {
+virBufferAsprintf(opt, ,iops=%llu,
+ disk-blkdeviotune.total_iops_sec);
+}
+
+if (disk-blkdeviotune.read_iops_sec) {
+virBufferAsprintf(opt, ,iops_rd=%llu,
+ disk-blkdeviotune.read_iops_sec);
+}
+
+if (disk-blkdeviotune.write_iops_sec) {
+virBufferAsprintf(opt, ,iops_wr=%llu,
+ disk-blkdeviotune.write_iops_sec);
+}
+}
+
if (virBufferError(opt)) {
virReportOOMError();
goto error;
diff --git a/src/util/xml.h b/src/util/xml.h
index c492063..5742f19 100644
--- a/src/util/xml.h
+++ b/src/util/xml.h
@@ -50,6 +50,8 @@ xmlNodePtr virXPathNode(const char *xpath,
int virXPathNodeSet(const char *xpath,
xmlXPathContextPtr ctxt,
xmlNodePtr **list);
+int virXMLStringToULongLong (const char *stringval,
+ unsigned long long *value);
char * virXMLPropString(xmlNodePtr node,
const char *name);
--
1.7.1
--
Adam Litke
Re: [libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
);
+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:),
reply.read_iops_sec);
+vshPrint(ctl, %-15s %llu\n, _(write_iops_sec:),
reply.write_iops_sec);
+
+virDomainFree(dom);
+return true;
+} else {
+
+ret = virDomainSetBlockIoTune(dom, disk, info, flags);
+
+if (ret == 0) {
+virDomainFree(dom);
+return true;
+}
+}
+
+out:
+virDomainFree(dom);
+return false;
+}
+
/*
* net-autostart command
@@ -14023,6 +14168,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},
+{blkdeviotune, cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune,
0},
#ifndef WIN32
{console, cmdConsole, opts_console, info_console, 0},
#endif
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 775d302..58fcb51 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -572,6 +572,29 @@ operation can be checked with Bblockjob.
Ipath specifies fully-qualified path of the disk.
Ibandwidth specifies copying bandwidth limit in Mbps.
+=item Bblkdeviotune Idomain Idevice [[I--total_bytes_sec
Btotal_bytes_sec]
+| [[I--read_bytes_sec Bread_bytes_sec] [I--write_bytes_sec
Bwrite_bytes_sec]]
+[[I--total_iops_sec Btotal_iops_sec] | [[I--read_iops_sec
Bread_iops_sec]
+[I--write_iops_sec Bwrite_iops_sec]] [[I--config] [I--live] |
[I--current]]
+
+Set or query the block disk io limits settting.
+Ipath specifies block disk name.
+I--total_bytes_sec specifies total throughput limit in bytes per second/s.
+I--read_bytes_sec specifies read throughput limit in bytes per second/s.
+I--write_bytes_sec specifies write throughput limit in bytes per second/s.
+I--total_iops_sec specifies total I/O operations limit per second/s.
+I--read_iops_sec specifies read I/O operations limit per second/s.
+I--write_iops_sec specifies write I/O operations limit per second/s.
+
+If I--live is specified, affect a running guest.
+If I--config is specified, affect the next boot of a persistent guest.
+If I--current is specified, affect the current guest state.
+Both I--live and I--current flags may be given, but I--current is
+exclusive. If no flag is specified, behavior is different depending
+on hypervisor.
+
+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
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] Support virDomain{Set, Get}BlockIoThrottle in the python API
= 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;
+
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(total_bytes_sec),
+ libvirt_ulonglongWrap(reply.total_bytes_sec));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(read_bytes_sec),
+ libvirt_ulonglongWrap(reply.read_bytes_sec));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(write_bytes_sec),
+ libvirt_ulonglongWrap(reply.write_bytes_sec));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(total_iops_sec),
+ libvirt_ulonglongWrap(reply.total_iops_sec));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(read_iops_sec),
+ libvirt_ulonglongWrap(reply.read_iops_sec));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(write_iops_sec),
+ libvirt_ulonglongWrap(reply.write_iops_sec));
+return ret;
+}
+
/***
* Helper functions to avoid importing modules
* for every callback
@@ -4837,6 +4920,8 @@ 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 *) virDomainSetBlockIoTune, libvirt_virDomainSetBlockIoTune,
METH_VARARGS, NULL},
+{(char *) virDomainGetBlockIoTune, libvirt_virDomainGetBlockIoTune,
METH_VARARGS, NULL},
{(char *) virDomainSendKey, libvirt_virDomainSendKey, METH_VARARGS,
NULL},
{(char *) virDomainMigrateGetMaxSpeed,
libvirt_virDomainMigrateGetMaxSpeed, METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
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
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?
Otherwise, it seems to work as advertised (based on my sniff testing).
Tested-by: Adam Litke a...@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 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
Re: [libvirt] [PATCH 1/8] Add new API virDomainSetBlockIoThrottle
(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+if (conn-driver-domainSetBlockIoThrottle) {
+int ret;
+ret = conn-driver-domainSetBlockIoThrottle(dom, disk, info, 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 9762fc4..ce29978 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -495,6 +495,7 @@ LIBVIRT_0.9.7 {
virDomainSnapshotGetParent;
virDomainSnapshotListChildrenNames;
virDomainSnapshotNumChildren;
+virDomainSetBlockIoThrottle;
} LIBVIRT_0.9.5;
# define new API here using predicted next version number
diff --git a/src/util/xml.h b/src/util/xml.h
index d30e066..cca3ea0 100644
--- a/src/util/xml.h
+++ b/src/util/xml.h
@@ -50,6 +50,8 @@ xmlNodePtr virXPathNode(const char *xpath,
int virXPathNodeSet(const char *xpath,
xmlXPathContextPtr ctxt,
xmlNodePtr **list);
+int virXMLStringToULongLong (const char *stringval,
+ unsigned long long *value);
char * virXMLPropString(xmlNodePtr node,
const char *name);
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] Add Block IO Throttle support setting to the remote driver
remote_procedure {
REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */
REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen
priority:high */
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen
autogen priority:high */
-REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248 /* skipgen skipgen */
+REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, /* skipgen skipgen */
+REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 249, /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 569fcb3..4f8bcf6 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -757,6 +757,17 @@ struct remote_domain_block_pull_args {
uint64_t bandwidth;
u_int flags;
};
+struct remote_domain_set_block_io_throttle_args {
+remote_nonnull_domain dom;
+remote_nonnull_string disk;
+uint64_t bps;
+uint64_t bps_rd;
+uint64_t bps_wr;
+uint64_t iops;
+uint64_t iops_rd;
+uint64_t iops_wr;
+u_int flags;
+};
struct remote_num_of_networks_ret {
intnum;
};
@@ -1999,4 +2010,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246,
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248,
+REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 249,
};
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] Implement Block Io Throttle setting for the qemu driver
On Thu, Oct 27, 2011 at 05:20:05PM +0800, Lei HH Li wrote:
+if (flags) {
+ret = virAsprintf(cmd, block_set_io_throttle %s %llu %llu %llu
%llu %llu %llu,
+ device,
+ info-bps,
+ info-bps_rd,
+ info-bps_wr,
+ info-iops,
+ info-iops_rd,
+ info-iops_wr);
Please combine the parameters onto fewer lines.
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] Add new API virDomainGetBlockIoThrottle
On Thu, Oct 27, 2011 at 05:20:06PM +0800, Lei HH Li wrote:
+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;
+}
You should be able to get the current settings with a read-only connection.
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] Getting Block IO Throttle support to remote driver
On Thu, Oct 27, 2011 at 05:20:07PM +0800, Lei HH Li wrote:
+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;
If not found, I would explicitly set found to 0. Not strictly necessary I know,
but I would rather not have correctness of the API depend on the ret structure
being zeroed out somewhere else.
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Implement Get Block IO Throttle for qemu driver
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.
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
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
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 -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
On Mon, Oct 17, 2011 at 02:08:42PM -0600, Jim Fehlig wrote: Hi Adam, I've been traveling, sorry for the delay... With the following config, I end up booting from the cdrom. If I take away the cdrom definition completely, I am unable to boot at all. I can boot fine with your config, either from sda (boot dev='hd' /) or cdrom (boot dev='cdrom' /). Have you verified your seabios supports booting from ahci as Bruce mentioned? No, I have not been able to verify it. Seabios refuses to build on my system with the version of binutils I have installed. I am fairly certain that I am using too old of a version of seabios (as you suggested). I am happy to withdraw my problem report. -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
On Tue, Oct 18, 2011 at 07:49:11AM -0500, Adam Litke wrote: On Mon, Oct 17, 2011 at 02:08:42PM -0600, Jim Fehlig wrote: Hi Adam, I've been traveling, sorry for the delay... With the following config, I end up booting from the cdrom. If I take away the cdrom definition completely, I am unable to boot at all. I can boot fine with your config, either from sda (boot dev='hd' /) or cdrom (boot dev='cdrom' /). Have you verified your seabios supports booting from ahci as Bruce mentioned? No, I have not been able to verify it. Seabios refuses to build on my system with the version of binutils I have installed. I am fairly certain that I am using too old of a version of seabios (as you suggested). I am happy to withdraw my problem report. And I just now was able to verify it. With an updated seabios, everything seems to be working fine for me. I can boot from the disks without problems. -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add a default event handle, to passthough the new events come from qemu
On Thu, Oct 13, 2011 at 10:54:13AM +0100, Daniel P. Berrange wrote: On Tue, Oct 11, 2011 at 08:46:06AM -0500, Adam Litke wrote: On Mon, Oct 10, 2011 at 11:54:08PM +0800, shao...@linux.vnet.ibm.com wrote: From: Shaohe Feng shao...@linux.vnet.ibm.com Basically, this feature can go along with qemu monitor passthrough. That way, if we use new commands in the monitor that generate new events, we want some way to receive those new events too. I agree that this patch is very complimentary to the existing tools for qemu interaction (qemu_monitor_command, qemu_attach, etc). It allows API users to subscribe to events that are not yet handled by libvirt. I have a couple of design questions about this feature. 1) This feature seems to be a bit qemu specific. Other qemu-specific APIs (mentioned above) are build into libvirt-qemu.so so that they are kept apart from the hypervisor-neutral parts of the API. Is it possible to do that for an event handler like this patch implements? Do we want to enforce such a limit? Yep, I agree it ought to be in the QEMU specific library. 2) This patch causes a string representing a raw JSON event object to be passed to the callbacks that are registered for the default event. This seems fine to me. I do wonder if relying on a 'default' event is a bad thing for an application to do. Let's say an app decides to handle NEW_EVENT using this default handler. Then, libvirt gains official support for NEW_EVENT. When the libvirt package is updated in the application's environment, NEW_EVENT will no longer trigger via the default handler. Thus, the application is broken by a libvirt upgrade. Would it be better to have a 'raw event' sink where all events (whether supported or not) would be sent? I expect applications will want the event to be dispatched to them, regardless of whether libvirt has handled it itself, so that they don't have to update themselves to the latest libvirt event API immediately. Also I think it would be better if we were able to connect to explicit JSON events, rather than a catch all. THe number of events from QEMU is only going to increase over time as it does, a 'catch all' becomes increasingly bandwidth wasteful. Agreed on both of these. I'd reckon we want something like this: typedef void (*virConnectDomainQemuEventCallback)(virConnectPtr conn, virDomainPtr dom, const char *eventName, /* The JSON event name */ const char *eventArgs, /* The JSON string of args */ void *opaque); virConnectDomainQemuEventRegister(virConnectPtr conn, virDomainPtr dom, /* option to filter */ const char *eventName, /* JSON event name */ virConnectDomainQemuEventCallback cb, void *opaque, virFreeCallback freecb); virConnectDomainQemuEventDeregister(virConnectPtr conn, int callbackID); in libvirt-qemu.h, and libvirt-qemu.so Looks good to me. -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 1/5] Add new API virDomainBlockIoThrottle
On Wed, Oct 12, 2011 at 03:49:26PM -0600, Eric Blake wrote: On 10/12/2011 01:07 AM, Zhi Yong Wu wrote: On Tue, Oct 11, 2011 at 10:59 PM, Adam Litkea...@us.ibm.com wrote: On Mon, Oct 10, 2011 at 09:45:09PM +0800, Lei HH Li wrote: Hi Lei. You are missing a patch summary at the top of this email. In your summary you want to let reviewers know what the patch is doing. For example, this patch defines the new virDomainBlockIoThrottle API and specifies the XML schema. Also at the top of the patch you have an opportunity to explain why you made a particular design decision. For example, you could explain why you chose I think so:). We should explain why we create one new libvirt commands, not extending blkiotune. BTW: Can we CCed these patches to those related developers to get their comments? (e.g, Daniel, Gui JianFeng, etc) to represent the throttling inside thedisk tag rather than alongside the blkiotune settings. I think the answer to this question lies in how it will be used. Looking at your patch, right now, we have: domain blkiotune weightnnn/weight /blkiotune /domain which is global to the domain, but blkio throttling is specified per-disk and can vary across multiple disks. So mention in your commit message that you are proposing a per-disk element, which is why it does not fit into the existing blkiotune: domain devices disk iothrottle .../ /disk /devices /domain Also, we need to agree on the final xml layout chosen - with 6 parameters, and the possibility of extension, while your patch did it all as attributes: iothrottle bps='nnn' bps_rd='nnn' .../ I'm thinking it would be better to use sub-elements (as was done with blkiotune/weight): iothrottle bpsnnn/bps bps_rdnnn/bps /iothrottle Also, is that naming the best? While qemu's command line might be terse, the XML should be something that reads well and which might even be portable to other hypervisors, so longer names might be more appropriate. Yes, good point Eric. I would also prefer to see these tags be expanded to more meaningful names. I propose: 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 -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 3/5] Implement virDomainBlockIoThrottle for the qemu driver
On Wed, Oct 12, 2011 at 03:02:12PM +0800, Zhi Yong Wu wrote:
On Tue, Oct 11, 2011 at 11:19 PM, Adam Litke a...@us.ibm.com wrote:
On Mon, Oct 10, 2011 at 09:45:11PM +0800, Lei HH Li wrote:
Summary here.
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) {
The above suggests that you should dynamically allocate the blkiothrottle
struct. Then you could reduce this check to:
If the structure is dynamically allocated, it will be easy to leak
memory although the checking is reduced.
Not using dynamic allocation because it is harder to do correctly is probably
not the best reasoning. There is a virDomainDiskDefFree() function to help free
dynamic memory in the disk definition. Anyway, there are also other ways to
clean this up. For example, you could add another field to disk-blkiothrottle
(.enabled?) to indicate whether throttling is active. Then you only have one
variable to check. For the record, I still prefer using a pointer to
blkiothrottle for this.
if (disk-blkiothrottle) {
+ 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
Re: [libvirt] [PATCH] add a default event handle, to passthough the new events come from qemu
On Mon, Oct 10, 2011 at 11:54:08PM +0800, shao...@linux.vnet.ibm.com wrote: From: Shaohe Feng shao...@linux.vnet.ibm.com Basically, this feature can go along with qemu monitor passthrough. That way, if we use new commands in the monitor that generate new events, we want some way to receive those new events too. I agree that this patch is very complimentary to the existing tools for qemu interaction (qemu_monitor_command, qemu_attach, etc). It allows API users to subscribe to events that are not yet handled by libvirt. I have a couple of design questions about this feature. 1) This feature seems to be a bit qemu specific. Other qemu-specific APIs (mentioned above) are build into libvirt-qemu.so so that they are kept apart from the hypervisor-neutral parts of the API. Is it possible to do that for an event handler like this patch implements? Do we want to enforce such a limit? 2) This patch causes a string representing a raw JSON event object to be passed to the callbacks that are registered for the default event. This seems fine to me. I do wonder if relying on a 'default' event is a bad thing for an application to do. Let's say an app decides to handle NEW_EVENT using this default handler. Then, libvirt gains official support for NEW_EVENT. When the libvirt package is updated in the application's environment, NEW_EVENT will no longer trigger via the default handler. Thus, the application is broken by a libvirt upgrade. Would it be better to have a 'raw event' sink where all events (whether supported or not) would be sent? In order to test this patch, see the attached python test case. When domains are started, it will be able to catch RESUME events. Tested-by: Adam Litke a...@us.ibm.com Signed-off-by: Shaohe Feng shao...@linux.vnet.ibm.com --- daemon/remote.c | 34 ++ include/libvirt/libvirt.h.in | 14 + python/libvirt-override-virConnect.py | 12 python/libvirt-override.c | 50 + src/conf/domain_event.c | 46 ++ src/conf/domain_event.h |5 +++ src/libvirt_private.syms |2 + src/qemu/qemu_monitor.c |9 ++ src/qemu/qemu_monitor.h |6 src/qemu/qemu_monitor_json.c | 31 src/qemu/qemu_process.c | 23 +++ src/remote/remote_driver.c| 31 src/remote/remote_protocol.x |8 - src/remote_protocol-structs |5 +++ 14 files changed, 275 insertions(+), 1 deletions(-) -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 1/5] Add new API virDomainBlockIoThrottle
(virDomainPtr dom,
+ const char *disk,
+ virDomainBlockIoThrottleInfoPtr info,
+ virDomainBlockIoThrottleInfoPtr reply,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(dom, disk=%p, info=%p, reply=%p,flags=%x,
+ disk, info, 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,
+ _(disk name is NULL));
We are using a newer error convention now. This should be:
virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
Be sure to fix all of these in your patch.
+goto error;
+}
+
+if (!reply) {
+virLibDomainError(VIR_ERR_INVALID_ARG,
+ _(reply is NULL));
+goto error;
+}
+
+if (conn-driver-domainBlockIoThrottle) {
+int ret;
+ret = conn-driver-domainBlockIoThrottle(dom, disk, info, 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 afea29b..0a79167 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -493,6 +493,7 @@ LIBVIRT_0.9.7 {
global:
virDomainReset;
virDomainSnapshotGetParent;
+virDomainBlockIoThrottle;
} LIBVIRT_0.9.5;
# define new API here using predicted next version number
diff --git a/src/util/xml.h b/src/util/xml.h
index d30e066..14b35b2 100644
--- a/src/util/xml.h
+++ b/src/util/xml.h
@@ -50,6 +50,9 @@ xmlNodePtr virXPathNode(const char *xpath,
int virXPathNodeSet(const char *xpath,
xmlXPathContextPtr ctxt,
xmlNodePtr **list);
+int virXMLStringToULongLong (const char *stringval,
+ unsigned long long *value);
+
char * virXMLPropString(xmlNodePtr node,
const char *name);
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 2/5] Add virDomainBlockIoThrottle support to the remote driver
*/
.domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
+.domainBlockIoThrottle = remoteDomainBlockIoThrottle, /* 0.9.4 */
Use the next planned version number (as of today: 0.9.8).
};
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 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_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;
+};
+
/* Network calls: */
struct remote_num_of_networks_ret {
@@ -2525,7 +2546,8 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */
-REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */
+REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */
+REMOTE_PROC_DOMAIN_BLOCK_IO_THROTTLE = 246 /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 69175cc..8f0cfcd 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -757,6 +757,25 @@ struct remote_domain_block_pull_args {
uint64_t bandwidth;
u_int flags;
};
+struct remote_domain_block_io_throttle_args {
+remote_nonnull_domain dom;
+remote_nonnull_string disk;
+uint64_t bps;
+uint64_t bps_rd;
+uint64_t bps_wr;
+uint64_t iops;
+uint64_t iops_rd;
+uint64_t iops_wr;
+u_int flags;
+};
+struct remote_domain_block_io_throttle_ret {
+uint64_t bps;
+uint64_t bps_rd;
+uint64_t bps_wr;
+uint64_t iops;
+uint64_t iops_rd;
+uint64_t iops_wr;
+};
struct remote_num_of_networks_ret {
intnum;
};
@@ -1970,5 +1989,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242,
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243,
REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244,
-REMOTE_PROC_DOMAIN_RESET = 245,
+REMOTE_PROC_DOMAIN_BLOCK_IO_THROTTLE = 245,
Oops, you removed REMOTE_PROC_DOMAIN_RESET :). Just append your new rpc ID.
};
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 4/5] Enable the virDomainBlockIoThrottle API in virsh
On Mon, Oct 10, 2011 at 09:45:12PM +0800, Lei HH Li wrote:
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}
+};
This command should also be able to display the current settings. My suggestion
would be that if none of the settings are named on the command line, the
behavior is to displaty the current settings. I can see that the command
documentation agrees with me and this is just a TODO. You should tell us that
in the patch summary :)
+
+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
--
Adam Litke a...@us.ibm.com
Re: [libvirt] [RFC PATCH 3/5] Implement virDomainBlockIoThrottle for the qemu driver
)
+{
+char *cmd = NULL;
+char *result = NULL;
+int ret = 0;
+
+if (flags) {
+ret = virAsprintf(cmd, block_set_io_throttle %s %llu %llu %llu
%llu %llu %llu,
+ device,
+ info-bps,
+ info-bps_rd,
+ info-bps_wr,
+ info-iops,
+ info-iops_rd,
+ info-iops_wr);
+} else {
+/*
+ret = virAsprintf(cmd, info block);
+*/
+}
+
+if (ret 0) {
+virReportOOMError();
+return -1;
+}
+
+if (qemuMonitorHMPCommand(mon, cmd, result) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(cannot run monitor command));
+ret = -1;
+goto cleanup;
+}
+
+if (0) {
+ret = qemuMonitorTextParseBlockIoThrottle(result, device, reply);
+}
+
+cleanup:
+VIR_FREE(cmd);
+VIR_FREE(result);
+return ret;
+}
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index cc2a252..66a23ac 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -243,4 +243,10 @@ int qemuMonitorTextSetLink(qemuMonitorPtr mon,
const char *name,
enum virDomainNetInterfaceLinkState state);
+int qemuMonitorTextBlockIoThrottle(qemuMonitorPtr mon,
+ const char *device,
+ virDomainBlockIoThrottleInfoPtr info,
+ virDomainBlockIoThrottleInfoPtr reply,
+ unsigned int flags);
+
#endif /* QEMU_MONITOR_TEXT_H */
--
1.7.1
--
Adam Litke a...@us.ibm.com
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
On Mon, Oct 03, 2011 at 10:46:18PM -0600, Jim Fehlig wrote: Adam Litke wrote: Hi Jim. I was testing this and found that I could not boot from the sata disks I defined. When I switch them back to ide, they can be booted just fine. Perhaps something is missing from the boot order logic? Hmm, I didn't notice this in my testing. sda in the below config contained /boot. Can you provide the domain config? With the following config, I end up booting from the cdrom. If I take away the cdrom definition completely, I am unable to boot at all. domain type='kvm' nameblockPull-test/name memory262144/memory currentMemory262144/currentMemory vcpu1/vcpu os type arch='x86_64' machine='pc-0.13'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/home/aglitke/src/qemu/x86_64-softmmu/qemu-system-x86_64/emulator disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk1.qed' / target dev='sda' bus='sata'/ /disk disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk2.qed' / target dev='sdb' bus='sata'/ /disk disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/home/aglitke/vm/images/natty-alternate-amd64.iso' / target dev='hda' bus='ide'/ /disk interface type=network source network=default / /interface graphics type='vnc' port='-1' autoport='yes'/ /devices /domain -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219136/memory + currentMemory219136/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' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='sda' bus='sata'/ + address type='drive' controller='0' bus='0' unit='0'/ +/disk +controller type='sata' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9e174b3..f298d37 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -359,6 +359,9 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-scsi-device-auto, false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +DO_TEST(disk-sata-device, false, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, +QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI); DO_TEST(disk-aio, false, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_AIO, QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT); -- 1.7.5.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-0.9.5 availability of rc2
I am getting SIGABRT and SIGSEGV in libvirtd when trying to catch blockJob events. When running under valgrind I get the following: ==19819== Thread 1: ==19819== Invalid free() / delete / delete[] ==19819==at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819==by 0x4E7B48: virFree (memory.c:310) ==19819==by 0x7669C32: virDomainEventFree (domain_event.c:510) ==19819==by 0x766AFE2: virDomainEventQueueDispatch (domain_event.c:1154) ==19819==by 0x766B19D: virDomainEventStateFlush (domain_event.c:1195) ==19819==by 0x483E15: qemuDomainEventFlush (qemu_domain.c:134) ==19819==by 0x507535: virEventPollRunOnce (event_poll.c:421) ==19819==by 0x4E6D44: virEventRunDefaultImpl (event.c:247) ==19819==by 0x44813C: virNetServerRun (virnetserver.c:701) ==19819==by 0x41FECE: main (libvirtd.c:1564) ==19819== Address 0x131b0a30 is 0 bytes inside a block of size 15 free'd ==19819==at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819==by 0x7FB006C: xdr_string (xdr.c:722) ==19819==by 0x43A5FD: xdr_remote_nonnull_string (remote_protocol.c:30) ==19819==by 0x442E2B: xdr_remote_domain_event_block_job_msg (remote_protocol.c:4000) ==19819==by 0x7FAF6C4: xdr_free (xdr.c:72) ==19819==by 0x431BDA: remoteRelayDomainEventBlockJob (remote.c:363) ==19819==by 0x766ADBA: virDomainEventDispatchDefaultFunc (domain_event.c:1079) ==19819==by 0x482C67: qemuDomainEventDispatchFunc (qemu_domain.c:125) ==19819==by 0x766AF3D: virDomainEventDispatch (domain_event.c:1136) ==19819==by 0x766AFD1: virDomainEventQueueDispatch (domain_event.c:1153) ==19819==by 0x766B19D: virDomainEventStateFlush (domain_event.c:1195) ==19819==by 0x483E15: qemuDomainEventFlush (qemu_domain.c:134) ==19819== On a different recreate under gdb I get: Program received signal SIGSEGV, Segmentation fault. malloc_consolidate (av=0x7f422020) at malloc.c:5155 5155malloc.c: No such file or directory. in malloc.c (gdb) bt #0 malloc_consolidate (av=0x7f422020) at malloc.c:5155 #1 0x7f422ef09528 in _int_free (av=0x7f422020, p=0x7f4220080f50) at malloc.c:5034 #2 0x7f422ef0d8e3 in __libc_free (mem=value optimized out) at malloc.c:3738 #3 0x004e7b29 in virFree (ptrptr=0x7fff5f07a458) at util/memory.c:310 #4 0x0044a1cf in virNetMessageFree (msg=0x7f4220080f60) at rpc/virnetmessage.c:69 #5 0x00445d4a in virNetServerClientDispatchWrite ( sock=value optimized out, events=2, opaque=0x7f422b90) at rpc/virnetserverclient.c:902 #6 virNetServerClientDispatchEvent (sock=value optimized out, events=2, opaque=0x7f422b90) at rpc/virnetserverclient.c:956 #7 0x00507787 in virEventPollDispatchHandles () at util/event_poll.c:470 #8 virEventPollRunOnce () at util/event_poll.c:611 #9 0x004e6d25 in virEventRunDefaultImpl () at util/event.c:247 #10 0x0044811d in virNetServerRun (srv=0x1dfec10) at rpc/virnetserver.c:701 #11 0x0041feaf in main (argc=value optimized out, argv=value optimized out) at libvirtd.c:1564 Looks like a double free somewhere. Commit: a91d3115b5c460af8a6f70d2092d0bc5ef9b723e seems to have surfaced the problem. On Thu, Sep 15, 2011 at 12:11:13AM +0800, Daniel Veillard wrote: I have made a second release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. I'm afraid we still didn't fix the MacOS-X / BSD problem , there is also some new code compared to rc1, so it's likely I will do an rc3 on Friday, and push the final release only beginning of last week. Please give it a try, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] BlockPull: Set initial bandwidth limit if specified
The libvirt BlockPull API supports the use of an initial bandwidth limit but the
qemu block_stream API does not. To get the desired behavior we use the two APIs
strung together: first BlockPull, then BlockJobSetSpeed. We can do this at the
driver level to avoid duplicated code in each monitor path.
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_driver.c |8 +++-
src/qemu/qemu_monitor_json.c |4
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4e8c691..7476bb4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9418,8 +9418,14 @@ static int
qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long
bandwidth,
unsigned int flags)
{
+int ret;
+
virCheckFlags(0, -1);
-return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_PULL);
+ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_PULL);
+if (ret == 0 bandwidth != 0)
+ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL,
+ BLOCK_JOB_SPEED);
+return ret;
}
static virDriver qemuDriver = {
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 715b26e..4ceb536 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2956,10 +2956,6 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
if (ret == 0 mode == BLOCK_JOB_INFO)
ret = qemuMonitorJSONGetBlockJobInfo(reply, device, info);
-if (ret == 0 mode == BLOCK_JOB_PULL bandwidth != 0)
-ret = qemuMonitorJSONBlockJob(mon, device, bandwidth, NULL,
- BLOCK_JOB_SPEED);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
--
1.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] BlockJob: Bandwidth parameter is in MB when using text monitor
Due to an unfortunate precedent in qemu, the units for the bandwidth parameter
to block_job_set_speed are different between the text monitor and the qmp
monitor. While the qmp monitor uses bytes/s, the text monitor expects MB/s.
Correct the units for the text interface.
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_monitor_text.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index f37c98c..854ee7f 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -3067,8 +3067,7 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon,
ret = virAsprintf(cmd, %s, cmd_name);
} else if (mode == BLOCK_JOB_SPEED) {
cmd_name = block_job_set_speed;
-ret = virAsprintf(cmd, %s %s %llu, cmd_name, device,
- bandwidth * 1024ULL * 1024ULL);
+ret = virAsprintf(cmd, %s %s %luM, cmd_name, device, bandwidth);
} else if (mode == BLOCK_JOB_PULL) {
cmd_name = block_stream;
ret = virAsprintf(cmd, %s %s, cmd_name, device);
--
1.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] BlockPull: Set bandwidth limit when using text monitor
The libvirt BlockPull API supports the use of an initial bandwidth limit. To implement this in the text monitor we first start the operation with a 'block_stream' command and then issue a 'block_job_set_speed' command to apply the limit. This functionality is already present for json mode. Signed-off-by: Adam Litke a...@us.ibm.com --- src/qemu/qemu_monitor_text.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 854ee7f..2159209 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3096,6 +3096,11 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextParseBlockJob(reply, device, info); +/* If non-zero, set the bandwidth limit with a separate command */ +if (ret == 0 mode == BLOCK_JOB_PULL bandwidth != 0) +ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, + BLOCK_JOB_SPEED); + cleanup: VIR_FREE(cmd); VIR_FREE(reply); -- 1.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
On 08/22/2011 10:04 PM, Osier Yang wrote:
Hi, Adam
I likes the idea to wrap the checking as a function seperately, but the
function
won't work well if the command is help info, though we don't have a use
of help info yet.
My point is since the function is intending to work for all the command,
as a
common function, it needs to consider some boudary too.
How about below?
Good points. The function below looks good.
qemuMonitorTextCommandNotFound(const char *cmd, const char *reply)
{
if (STRPREFIX(cmd, info )) {
if (strstr(reply, info version))
return 1;
} else {
if (strstr(reply, unknown command:))
return 1;
}
return 0;
}
And there might be other different info qemu will output for a unknown
command
we don't known yet. Using cmd as an argument will allow us to extend
the checking
methods.
Thanks
Osier
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] blockjob: Report correct error when missing qemu support
When executing blockjob monitor commands, we are not checking for
CommandNotFound errors. This results in 'unexpected error' messages when using
a qemu without support for the required commands. Fix this up by checking for
CommandNotFound errors for QMP and detecting this condition for the text
monitor.
This fixes the problem reported in Bug 727502.
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_monitor_json.c |3 +++
src/qemu/qemu_monitor_text.c | 20
2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7adfb26..4c8a08b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2939,6 +2939,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
else if (qemuMonitorJSONHasError(reply, NotSupported))
qemuReportError(VIR_ERR_OPERATION_INVALID,
_(Operation is not supported for device: %s), device);
+else if (qemuMonitorJSONHasError(reply, CommandNotFound))
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(Operation is not supported));
else
qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unexpected error));
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 52d924a..d9cad2d 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -271,6 +271,19 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon,
scm_fd, reply);
}
+/* Check monitor output for evidence that the command was not recognized.
+ * For 'info' commands, qemu returns help text. For other commands, qemu
+ * returns 'unknown command:'.
+ */
+static int
+qemuMonitorTextCommandNotFound(const char *reply)
+{
+if (strstr(reply, unknown command:))
+return 1;
+if (strstr(reply, info version -- show the version of QEMU))
+return 1;
+return 0;
+}
static int
qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon,
@@ -3056,6 +3069,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon,
goto cleanup;
}
+if (qemuMonitorTextCommandNotFound(reply)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(Operation is not supported));
+ret = -1;
+goto cleanup;
+}
+
ret = qemuMonitorTextParseBlockJob(reply, device, info);
cleanup:
--
1.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
block-jobs command.
+ * If qemu monitor command info doesn't support one sub-command,
+ * it will print the output of help info instead, so simply check
+ * if info version is in the output.
+ */
+if (mode == BLOCK_JOB_INFO) {
+if (strstr(reply, info version)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+_(Command '%s' is not found), cmd_name);
+ret = -1;
+goto cleanup;
+}
+} else {
+if (strstr(reply, unknown command)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+_(Command '%s' is not found), cmd_name);
+ret = -1;
+goto cleanup;
+}
+}
+
ret = qemuMonitorTextParseBlockJob(reply, device, info);
cleanup:
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
From: Osier Yang jy...@redhat.com
* src/qemu/qemu_monitor_json.c: Handle error CommandNotFound and
report the error.
* src/qemu/qemu_monitor_text.c: If a sub info command is not found,
it prints the output of help info, for other commands,
unknown command is printed.
Without this patch, libvirt always report:
An error occurred, but the cause is unknown
This patch was adapted from a patch by Osier Yang jy...@redhat.com to break
out detection of unrecognized text monitor commands into a separate function.
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_monitor_json.c | 34 ++---
src/qemu/qemu_monitor_text.c | 47 +-
2 files changed, 58 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7adfb26..715b26e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
-
-if (mode == BLOCK_JOB_ABORT)
-cmd = qemuMonitorJSONMakeCommand(block_job_cancel,
- s:device, device, NULL);
-else if (mode == BLOCK_JOB_INFO)
-cmd = qemuMonitorJSONMakeCommand(query-block-jobs, NULL);
-else if (mode == BLOCK_JOB_SPEED)
-cmd = qemuMonitorJSONMakeCommand(block_job_set_speed,
- s:device, device,
- U:value, bandwidth * 1024ULL *
1024ULL,
+const char *cmd_name = NULL;
+
+if (mode == BLOCK_JOB_ABORT) {
+cmd_name = block_job_cancel;
+cmd = qemuMonitorJSONMakeCommand(cmd_name, s:device, device, NULL);
+} else if (mode == BLOCK_JOB_INFO) {
+cmd_name = query-block-jobs;
+cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL);
+} else if (mode == BLOCK_JOB_SPEED) {
+cmd_name = block_job_set_speed;
+cmd = qemuMonitorJSONMakeCommand(cmd_name, s:device,
+ device, U:value,
+ bandwidth * 1024ULL * 1024ULL,
NULL);
-else if (mode == BLOCK_JOB_PULL)
-cmd = qemuMonitorJSONMakeCommand(block_stream,
- s:device, device, NULL);
+} else if (mode == BLOCK_JOB_PULL) {
+cmd_name = block_stream;
+cmd = qemuMonitorJSONMakeCommand(cmd_name, s:device,
+ device, NULL);
+}
if (!cmd)
return -1;
@@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
else if (qemuMonitorJSONHasError(reply, NotSupported))
qemuReportError(VIR_ERR_OPERATION_INVALID,
_(Operation is not supported for device: %s), device);
+else if (qemuMonitorJSONHasError(reply, CommandNotFound))
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+_(Command '%s' is not found), cmd_name);
else
qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unexpected error));
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 52d924a..9119f06 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -271,6 +271,20 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon,
scm_fd, reply);
}
+/* Check monitor output for evidence that the command was not recognized.
+ * For 'info' commands, qemu returns help text. For other commands, qemu
+ * returns 'unknown command:'.
+ */
+static int
+qemuMonitorTextCommandNotFound(const char *reply)
+{
+if (strstr(reply, unknown command:))
+return 1;
+if (strstr(reply, info version))
+return 1;
+return 0;
+}
+
static int
qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon,
@@ -3031,18 +3045,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon,
char *cmd = NULL;
char *reply = NULL;
int ret;
-
-if (mode == BLOCK_JOB_ABORT)
-ret = virAsprintf(cmd, block_job_cancel %s, device);
-else if (mode == BLOCK_JOB_INFO)
-ret = virAsprintf(cmd, info block-jobs);
-else if (mode == BLOCK_JOB_SPEED)
-ret = virAsprintf(cmd, block_job_set_speed %s %llu, device,
+const char *cmd_name = NULL;
+
+if (mode == BLOCK_JOB_ABORT) {
+cmd_name = block_job_cancel;
+ret = virAsprintf(cmd, %s %s, cmd_name, device);
+} else if (mode == BLOCK_JOB_INFO) {
+cmd_name = info block-jobs;
+ret = virAsprintf(cmd, %s, cmd_name);
+} else if (mode == BLOCK_JOB_SPEED) {
+cmd_name = block_job_set_speed;
+ret = virAsprintf(cmd, %s %s %llu, cmd_name, device,
bandwidth * 1024ULL * 1024ULL);
-else if (mode == BLOCK_JOB_PULL)
-ret = virAsprintf(cmd, block_stream %s, device
Re: [libvirt] [RFC] NUMA topology specification
On 08/19/2011 01:35 AM, Bharata B Rao wrote: May be something like this: (OPTION 2) cpu ... topology sockets='1' cores='2' threads='1' nodeid='0' cpus='0-1' mem='size' topology sockets='1' cores='2' threads='1' nodeid='1' cpus='2-3' mem='size' ... /cpu This should result in a 2 node system with each node having 1 socket with 2 cores. Comments, suggestions ? Option 2 (above) seems like the most logical interface to me. I would not support putting this under numatune because of the high risk of users confusing guest NUMA topology definition with host NUMA tuning. I like the idea of merging this into topology to prevent errors with specifying incompatible cpu and numa topologies but I think you can go a step further (assuming my following assertion is valid). Since cpus are assigned to numa nodes at the core level, and you are providing a 'nodeid' attribute, you can infer the 'cpus' attribute using 'cores' and 'nodeid' alone. For your example above: topology sockets='1' cores='2' threads='1' nodeid='0' mem='size' topology sockets='1' cores='2' threads='1' nodeid='1' mem='size' You have 4 cores total, each node is assigned 2. Assign cores to nodes starting with core 0 and node 0. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] latency: Introduce new members for _virDomainBlockStats
On 08/17/2011 12:15 PM, Eric Blake wrote: I just audited libvirt.c, and found that the following libvirt interfaces that take a size parameter, and could be extensible by use of the size field: virDomainBlockStats virDomainInterfaceStats The following libvirt interfaces take a flags parameter, and could be extensible by setting a flag bit that says a larger struct is in use: virDomainGetControlInfo virNodeGetCPUStats virNodeGetMemoryStats virDomainMemoryStats virDomainMemoryStats() was designed to be regularly extended by using the 'nr_stats' parameter. When I added this API, we had this discusson and the design emerged specifically for the kind of extension being discussed here. If a newer client talks to an older server, the effect is that the client asks for more stats than are available. The old server simply returns the stats it knows about. Wouldn't this be the best approach to use for block stats as well? Am I missing a downside to the virDomainMemoryStats() approach? virDomainGetBlockInfo virDomainGetBlockJobInfo Meanwhile, the following APIs which take a pointer to a struct are non-extensible (neither size nor flags as an escape hatch to indicate explicit use of a larger struct), extending any of these would require a new API function: virDomainGetInfo virNodeGetInfo virDomainGetVcpus virDomainGetSecurityLabel virDomainGetSecurityModel virStoragePoolGetInfo virStorageVolGetInfo virDomainGetJobInfo -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On 08/15/2011 10:28 PM, Osier Yang wrote:
于 2011年08月16日 00:40, Adam Litke 写道:
Hi Osier,
Just to be clear, this is a cleanup not a bugfix right? The current
code should be working properly as written.
No, virDomainMemoryStats will return empty result if libvirt
communicates to qemu monitor in text mode. Actually this
is reported by a upstream Debian user, It seems to me packager
of Debian compiles libvirt without QMP support.
Ok, I am now understanding what you are trying to do. Originally, the
virDomainMemoryStats API did not include the 'actual' field. At the
time it seemed redundant to include it since that information is already
returned by virDomainGetInfo(). However, I am not opposed to including
it here.
On 08/15/2011 08:58 AM, Osier Yang wrote:
* src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as
balloon: actual=, which cause actual= is stripped early before
the real parsing. This patch changes BALLOON_PREFIX into balloon: ,
and modifies related functions, also renames
qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo,
as after the changing, it parses all the info returned by info
balloon.
---
src/qemu/qemu_monitor_text.c | 47
+
1 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 7bf733d..d17d863 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned
int tag,
return 0;
}
-/* Convert bytes to kilobytes for libvirt */
switch (tag) {
+/* Convert megabytes to kilobytes for libvirt */
+case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON:
+value = value 10;
+break;
+/* Convert bytes to kilobytes for libvirt */
case VIR_DOMAIN_MEMORY_STAT_SWAP_IN:
case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT:
case VIR_DOMAIN_MEMORY_STAT_UNUSED:
@@ -565,15 +569,17 @@ static int parseMemoryStat(char **text,
unsigned int tag,
/* The reply from the 'info balloon' command may contain additional
memory
* statistics in the form: '[,tag=val]*'
*/
-static int qemuMonitorParseExtraBalloonInfo(char *text,
-virDomainMemoryStatPtr
stats,
-unsigned int nr_stats)
+static int qemuMonitorParseBalloonInfo(char *text,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats)
{
char *p = text;
unsigned int nr_stats_found = 0;
while (*p nr_stats_found nr_stats) {
-if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN,
+if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON,
+actual=,stats[nr_stats_found]) ||
+parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN,
Also, don't forget to edit the comment at the top of the function. The
expected format is now: actual=val[,tag=val]*
,mem_swapped_in=,stats[nr_stats_found]) ||
parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT,
,mem_swapped_out=,stats[nr_stats_found]) ||
According to this code (and actual monitor behavior) 'actual' always
appears and has to come first. Therefore, I would still parse the
'actual' stat outside of the while loop.
@@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char
*text,
parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_UNUSED,
,free_mem=,stats[nr_stats_found]) ||
parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE,
-,total_mem=,stats[nr_stats_found]) ||
-parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON,
-,actual=,stats[nr_stats_found]))
+,total_mem=,stats[nr_stats_found]))
nr_stats_found++;
/* Skip to the next label. When *p is ',' the last match
attempt
@@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char
*text,
/* The reply from QEMU contains 'ballon: actual=421' where value is
in MB */
-#define BALLOON_PREFIX balloon: actual=
+#define BALLOON_PREFIX balloon:
/*
* Returns: 0 if balloon not supported, +1 if balloon query worked
@@ -625,13 +629,22 @@ int
qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
unsigned int memMB;
char *end;
offset += strlen(BALLOON_PREFIX);
-if (virStrToLong_ui(offset,end, 10,memMB) 0) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-_(could not parse memory balloon
allocation from '%s'), reply);
-goto cleanup;
+
+if (STRPREFIX(offset, actual
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
Stefan has a git repo with QED block streaming support here: git://repo.or.cz/qemu/stefanha.git stream-command On 08/15/2011 08:25 PM, Zhi Yong Wu wrote: On Mon, Aug 15, 2011 at 8:36 PM, Adam Litke a...@us.ibm.com wrote: On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. Sure, If you share it with me, at the same time learning your libvirt API, i can also sample this feature.:) anyway, thanks, Adam. -- Adam Litke IBM Linux Technology Center -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
support ballooning
Why not just use qemuMonitorParseBalloonInfo() for the logic above? You
only need to request one stat since actual must be first.
@@ -660,9 +673,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon,
if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) {
offset += strlen(BALLOON_PREFIX);
-if ((offset = strchr(offset, ',')) != NULL) {
-ret = qemuMonitorParseExtraBalloonInfo(offset, stats, nr_stats);
-}
+ret = qemuMonitorParseBalloonInfo(offset, stats, nr_stats);
}
VIR_FREE(reply);
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On 08/15/2011 11:50 AM, Daniel P. Berrange wrote: On Mon, Aug 15, 2011 at 11:27:43AM -0500, Adam Litke wrote: On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. I'd completely forgotten about this problem. We really should try to get this fixed renabled in QEMU sometime in the not too distant future. I agree. The problem is that qemu lacks a proper interface for asynchronous commands so an unresponsive guest could freeze the monitor. QMP is undergoing a significant overhaul as a result of the new QAPI framework. It is my understanding that this new framework will provide a robust async interface, allowing us to re-enable balloon stats. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] doc: Document the Block Job API
Hi DV,
This patch adds some basic documentation to the development guide for the Block
Job APIs as you requested. Also included is a sample program that shows
end-to-end use of the BlockPull operation. I hope this is close to what you
had in mind.
Signed-off-by: Adam Litke a...@us.ibm.com
---
en-US/Guest_Domains.xml | 154 +++
1 files changed, 154 insertions(+), 0 deletions(-)
diff --git a/en-US/Guest_Domains.xml b/en-US/Guest_Domains.xml
index 0324e78..202881d 100644
--- a/en-US/Guest_Domains.xml
+++ b/en-US/Guest_Domains.xml
@@ -1219,6 +1219,160 @@ fprintf(stderr, Device is suitable for passthrough to
a guest\n);
/section
+section id=Application_Development_Guide-Live_Config-Block_Jobs
+ titleBlock Device Jobs/title
+
+ para
+Libvirt provides a generic Block Job API that can be used to initiate
+and manage operations on disks that belong to a domain. Jobs are
+started by calling the function associated with the desired operation
+(eg. literalvirDomainBlockPull/literal). Once started, all block
+jobs are managed in the same manner. They can be aborted, throttled,
+and queried. Upon completion, an asynchronous event is issued to
+indicate the final status.
+ /para
+
+ para
+The following block jobs can be started:
+ /para
+ orderedlist
+listitem
+ para
+ literalvirDomainBlockPull()/literal starts a block pull
+operation for the specified disk. This operation is valid only for
+specially configured disks. BlockPull will populate a disk image
+with data from its backing image. Once all data from its backing
+image has been pulled, the disk no longer depends on a backing
+image.
+ /para
+/listitem
+ /orderedlist
+
+ para
+A disk can be queried for active block jobs by using
+literalvirDomainGetBlockJobInfo()/literal. If found, job
+information is reported in a structure that contains: the job type,
+bandwidth throttling setting, and progress information.
+ /para
+
+ para
+literalvirDomainBlockJobAbort()/literal can be used to cancel the
+active block job on the specified disk.
+ /para
+
+ para
+Use literalvirDomainBlockJobSetSpeed()/literal to limit the amount
+of bandwidth that a block job may consume. Bandwidth is specified in
+units of MB/sec.
+ /para
+
+ para
+When a block job operation completes, the final status is reported
using
+an asynchronous event. To receive this event, register a
+literalvirConnectDomainEventBlockJobCallback/literal function which
+will receive the disk, event type, and status as parameters.
+ /para
+
+ programlisting
+![CDATA[/* example blockpull-example.c */
+/* compile with: gcc -g -Wall blockpull-example.c -o blockpull-example -lvirt
*/
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include libvirt/libvirt.h
+
+int do_cmd(const char *cmdline)
+{
+int status = system(cmdline);
+if (status 0)
+return -1;
+else
+return WEXITSTATUS(status);
+}
+
+virDomainPtr make_domain(virConnectPtr conn)
+{
+virDomainPtr dom;
+char domxml[] = \
+ domain type='kvm' \
+ nameexample/name \
+ memory131072/memory \
+ vcpu1/vcpu \
+ os \
+type arch='x86_64' machine='pc-0.13'hvm/type \
+ /os \
+ devices \
+disk type='file' device='disk' \
+ driver name='qemu' type='qed'/ \
+ source file='/var/lib/libvirt/images/example.qed' / \
+ target dev='vda' bus='virtio'/ \
+/disk \
+ /devices \
+/domain;
+
+do_cmd(qemu-img create -f raw /var/lib/libvirt/images/backing.qed 100M);
+do_cmd(qemu-img create -f qed -b /var/lib/libvirt/images/backing.qed \
+/var/lib/libvirt/images/example.qed);
+
+dom = virDomainCreateXML(conn, domxml, 0);
+return dom;
+}
+
+int main(int argc, char *argv[])
+{
+virConnectPtr conn;
+virDomainPtr dom = NULL;
+char disk[] = /var/lib/libvirt/images/example.qed;
+
+conn = virConnectOpen(qemu:///system);
+if (conn == NULL) {
+fprintf(stderr, Failed to open connection to qemu:///system\n);
+goto error;
+}
+
+dom = make_domain(conn);
+if (dom == NULL) {
+fprintf(stderr, Failed to create domain\n);
+goto error;
+}
+
+if ((virDomainBlockPull(dom, disk, 0, 0)) 0) {
+fprintf(stderr, Failed to start block pull);
+goto error;
+}
+
+while (1) {
+virDomainBlockJobInfo info;
+int ret = virDomainGetBlockJobInfo(dom, disk, info, 0);
+
+if (ret == 1) {
+printf(BlockPull progress: %0.0f %%\n,
+(float)(100
Re: [libvirt] [PATCH] Fix bug #611823 prohibit pools with duplicate 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; More whitespace problems here. @@ -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; ... and here. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND RFC v4 1/6] Introduce the function virCgroupForVcpu
On 07/22/2011 01:56 AM, Wen Congyang wrote: So current behaviour if vCPU threads set quota in vCPU group else set nVCPUs * quota in VM group Would change to set nVCPUs * quota in VM group if vCPU threads set quota in vCPU group ? Yes. I treat this answer as you agree with Daniel P. Berrange's idea. If so, I will implement it. Yes, I am in agreement. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
Thanks Daniel. The upstream code is looking good. I will work on adding some documentation to the development guide. On 07/22/2011 01:07 AM, Daniel Veillard wrote: On Thu, Jul 21, 2011 at 01:55:04PM -0500, Adam Litke wrote: Here are the patches to implement the BlockPull/BlockJob API as discussed and agreed to. I am testing with a python script (included for completeness as the final patch). The qemu monitor interface is not expected to change in the future. Stefan is planning to submit placeholder commands for upstream qemu until the generic streaming support is implemented. Changes since V1: - Make virDomainBlockPullAbort() and virDomainGetBlockPullInfo() into a generic BlockJob interface. - Added virDomainBlockJobSetSpeed() - Rename VIR_DOMAIN_EVENT_ID_BLOCK_PULL event to fit into block job API - Add bandwidth argument to virDomainBlockPull() Summary of changes since first generation patch series: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names -- To help speed the provisioning process for large domains, new QED disks are created with backing to a template image. These disks are configured with copy on read such that blocks that are read from the backing file are copied to the new disk. This reduces I/O over a potentially costly path to the backing image. In such a configuration, there is a desire to remove the dependency on the backing image as the domain runs. To accomplish this, qemu will provide an interface to perform sequential copy on read operations during normal VM operation. Once all data has been copied, the disk image's link to the backing file is removed. The virDomainBlockPull API family brings this functionality to libvirt. virDomainBlockPull() instructs the hypervisor to stream the entire device in the background. Progress of this operation can be checked with the function virDomainBlockJobInfo(). An ongoing stream can be cancelled with virDomainBlockJobAbort(). virDomainBlockJobSetSpeed() allows you to limit the bandwidth that the operation may consume. An event (VIR_DOMAIN_EVENT_ID_BLOCK_JOB) will be emitted when a disk has been fully populated or if a BlockPull() operation was terminated due to an error. This event is useful to avoid polling on virDomainBlockJobInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. Thanks Adam for that revised patch set. ACK It all looked good to me, based on previous review and a last look. I just had to fix a few merge conflicts due to new entry points being added in the meantime and one commit message, but basically it was clean :-) So I pushed the set except 8 of course. I'm not sure if we should try to store it in the example, or on the wiki. The Wiki might be a bit more logical because I'm not sure we can run the test as is now in all setups. I think the remaining item would be to add documentation about how to use this, the paragraphs above should probably land somewhere on the web site, ideally on the development guide http://libvirt.org/devguide.html but I'm open to suggestions :-) Daniel -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND RFC v4 1/6] Introduce the function virCgroupForVcpu
Added Anthony to give him the opportunity to address the finer points of this one especially with respect to the qemu IO thread(s). This feature is really about capping the compute performance of a VM such that we get consistent top end performance. Yes, qemu has non-VCPU threads that this patch set doesn't govern, but that's the point. We are not attempting to throttle IO or device emulation with this feature. It's true that an IO-intensive guest may consume more host resources than a compute intensive guest, but they should still have equal top-end CPU performance when viewed from the guest's perspective. On 07/21/2011 05:09 AM, Daniel P. Berrange wrote: On Thu, Jul 21, 2011 at 10:08:03AM +0800, Wen Congyang wrote: Introduce the function virCgroupForVcpu() to create sub directory for each vcpu. I'm far from convinced that this is a good idea. Setting policies on individual VCPUs is giving the host admin a misleading illusion of control over individual guest VCPUs. The problem is that QEMU has many non-VCPU threads which consume non-trivial CPU time. CPU time generated by a CPU in the guest, does not neccessarily map to a VCPU thread in the host, but could instead go to a I/O thread or a display thread, etc. IMHO we should only be doing controls at the whole VM level here. Daniel -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND RFC v4 5/6] qemu: Implement cfs_period and cfs_quota's modification
On 07/20/2011 09:11 PM, Wen Congyang wrote:
+static int
+qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
+ unsigned long long *period, long long *quota)
+{
+virCgroupPtr cgroup_vcpu = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+int rc;
+int ret = -1;
+
+priv = vm-privateData;
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* We do not create sub dir for each vcpu */
+rc = qemuGetVcpuBWLive(cgroup, period, quota);
+if (rc 0)
+goto cleanup;
+
+if (*quota 0)
+*quota /= vm-def-vcpus;
+goto out;
+}
Are you sure the above is correct? Based on my earlier suggestion,
quota is always specified as the amount of runtime afforded to a
single vcpu. Hence, if you are changing quota to cover for all of a
vm's vcpus, wouldn't you want to:
*quota *= vm-def-vcpus;
?
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND RFC v4 5/6] qemu: Implement cfs_period and cfs_quota's modification
On 07/21/2011 06:01 AM, Daniel P. Berrange wrote:
On Thu, Jul 21, 2011 at 10:11:32AM +0800, Wen Congyang wrote:
@@ -5851,7 +5953,6 @@ static int
qemuSetSchedulerParametersFlags(virDomainPtr dom,
virTypedParameterPtr param = params[i];
if (STREQ(param-field, cpu_shares)) {
-int rc;
if (param-type != VIR_TYPED_PARAM_ULLONG) {
qemuReportError(VIR_ERR_INVALID_ARG, %s,
_(invalid type for cpu_shares tunable,
expected a 'ullong'));
@@ -5870,19 +5971,47 @@ static int
qemuSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-persistentDef = virDomainObjGetPersistentDef(driver-caps,
vm);
-if (!persistentDef) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
-_(can't get persistentDef));
+vmdef-cputune.shares = params[i].value.ul;
+}
+} else if (STREQ(param-field, cfs_period)) {
[snip]
+} else if (STREQ(param-field, cfs_quota)) {
In the XML file we now have
cputune
shares1024/shares
period9/period
quota0/quota
/cputune
But the schedinfo parameter are being named
cpu_shares: 1024
cfs_period: 9
cfs_quota: 0
These new tunables should be named 'cpu_period' and 'cpu_quota' too.
You mean 'cfs_period' and 'cfs_quota', right? The only problem I see
with that naming (specifically for the quota) is that it implies a
direct relationship to the tunable in cgroupfs. While true for
'cpu_shares', it is not true for quota since the proper setting to apply
depends on the threading behavior of qemu.
+static int
+qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
+ unsigned long long *period, long long *quota)
+{
+virCgroupPtr cgroup_vcpu = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+int rc;
+int ret = -1;
+
+priv = vm-privateData;
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* We do not create sub dir for each vcpu */
+rc = qemuGetVcpuBWLive(cgroup, period, quota);
+if (rc 0)
+goto cleanup;
+
+if (*quota 0)
+*quota /= vm-def-vcpus;
+goto out;
+}
+
+/* get period and quota for vcpu0 */
+rc = virCgroupForVcpu(cgroup, 0, cgroup_vcpu, 0);
+if (!cgroup_vcpu) {
+virReportSystemError(-rc,
+ _(Unable to find vcpu cgroup for %s(vcpu:
0)),
+ vm-def-name);
+goto cleanup;
+}
+
+rc = qemuGetVcpuBWLive(cgroup_vcpu, period, quota);
+if (rc 0)
+goto cleanup;
This is also bad IMHO, giving different semantics for the operation
of the API, depending on whether the QEMU impl has VCPUs threads or
not. Again this just says to me that we need this to apply at the VM
level not the VCPU level.
IMO, this firms up the semantics of the libvirt API. As these patches
are written, the libvirt semantics are: Apply cpu capping to this
domain. Period is the measurement interval. Quota is the amount of
time each vcpu may run within the period. We are able to insulate the
users from details of the underlying qemu -- whether it supports threads
or not.
Another reason a per-vcpu quota setting is more desirable than a global
setting is seen when changing the number of vcpus assigned to a domain.
Each time you change the vcpu count, you would have to change the quota
number too. Imagine all of the complaints from users when they increase
the vcpu count and find their domain actually performs worse.
If we really do need finer per-VCPU controls, in addition to a per-VM
control, I think that should likely be done as a separate tunable,
or separate CPU
eg, we could have 2 sets of tunables, one that applies to the VM and
the second set that adds further controls to the VCPUs.
How would this behave when qemu doesn't support vcpu threads? We'd
either have to throw an error, or just ignore the vcpu_ settings.
either way, you're exposing users to qemu internals. They would need to
change their domain.xml files when moving from a multi-threaded qemu to
a single-threaded qemu.
cputune
shares1024/shares
period9/period
quota0/quota
vcpu_shares1024/vcpu_shares
vcpu_period9/vcpu_period
vcpu_quota0/vcpu_quota
/cputune
I actually do like the vcpu_* naming because it makes two things more
clear: Units apply at the vcpu level, and these stats don't necessarily
map directly to cgroupfs. However, I would still maintain that the
current logic should be used for applying the settings (per-vcpu if
supported, multiply by nvcpus and apply globally if vcpu threads are not
supported).
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https
Re: [libvirt] [PATCH RESEND RFC v4 1/6] Introduce the function virCgroupForVcpu
On 07/21/2011 08:34 AM, Daniel P. Berrange wrote: On Thu, Jul 21, 2011 at 07:54:05AM -0500, Adam Litke wrote: Added Anthony to give him the opportunity to address the finer points of this one especially with respect to the qemu IO thread(s). This feature is really about capping the compute performance of a VM such that we get consistent top end performance. Yes, qemu has non-VCPU threads that this patch set doesn't govern, but that's the point. We are not attempting to throttle IO or device emulation with this feature. It's true that an IO-intensive guest may consume more host resources than a compute intensive guest, but they should still have equal top-end CPU performance when viewed from the guest's perspective. I could be mis-understanding, what you're trying to achieve, here, so perhaps we should consider an example. From your example, it's clear to me that you understand the use case well. - A machine has 4 physical CPUs - There are 4 guests on the machine - Each guest has 2 virtual CPUs So we've overcommit the host CPU resources x2 here. Lets say that we want to use this feature to ensure consistent top end performance of every guest, splitting the host pCPUs resources evenly across all guests, so each guest is ensured 1 pCPU worth of CPU time overall. This patch lets you do this by assigning caps per VCPU. So in this example, each VCPU cgroup would have to be configured to cap the VCPUs at 50% of a single pCPU. This leaves the other QEMU threads uncapped / unaccounted for. If any one guest causes non-trivial compute load in a non-VCPU thread, this can/will impact the top-end compute performance of all the other guests on the machine. If we did caps per VM, then you could set the VM cgroup such that the VM as a whole had 100% of a single pCPU. If a guest is 100% compute bound, it can use its full 100% of a pCPU allocation in vCPU threads. If any other guest is causing CPU time in a non-VCPU thread, it cannot impact the top end compute performance of VCPU threads in the other guests. A per-VM cap would, however, mean a guest with 2 vCPUs could have unequal scheduling, where one vCPU claimed 75% of the pCPU and the othe vCPU got left with only 25%. So AFAICT, per-VM cgroups is better for ensuring top end compute performance of a guest as a whole, but per-VCPU cgroups can ensure consistent top end performance across vCPUs within a guest. IMHO, per-VM cgroups is the more useful because it is the only way to stop guests impacting each other, but there could be additional benefits of *also* have per-VCPU cgroups if you want to ensure fairness of top-end performance across vCPUs inside a single VM. What this says to me is that per-VM cgroups _in_addition_to_ per-vcpu cgroups is the _most_ useful situation. Since I can't think of any cases where someone would want per-vm and not per-vcpu, how about we always do both when supported. We can still use one pair of tunables (period and quota) and try to do the right thing. For example: vcpus2/vcpus cputune period50/period quota25/quota /cputune Would have the following behavior for qemu-kvm (vcpu threads) Global VM cgroup: cfs_period:50 cfs_quota:50 Each vcpu cgroup: cfs_period:50 cfs_quota:25 and this behavior for qemu with no vcpu threads Global VM cgroup: cfs_period:50 cfs_quota:50 It's true that IO could still throw off the scheduling balance somewhat among vcpus _within_ a VM, but this effect would be confined within the vm itself. Best of both worlds? -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND RFC v4 1/6] Introduce the function virCgroupForVcpu
On 07/21/2011 09:29 AM, Daniel P. Berrange wrote: On Thu, Jul 21, 2011 at 08:49:28AM -0500, Adam Litke wrote: On 07/21/2011 08:34 AM, Daniel P. Berrange wrote: On Thu, Jul 21, 2011 at 07:54:05AM -0500, Adam Litke wrote: Added Anthony to give him the opportunity to address the finer points of this one especially with respect to the qemu IO thread(s). This feature is really about capping the compute performance of a VM such that we get consistent top end performance. Yes, qemu has non-VCPU threads that this patch set doesn't govern, but that's the point. We are not attempting to throttle IO or device emulation with this feature. It's true that an IO-intensive guest may consume more host resources than a compute intensive guest, but they should still have equal top-end CPU performance when viewed from the guest's perspective. I could be mis-understanding, what you're trying to achieve, here, so perhaps we should consider an example. From your example, it's clear to me that you understand the use case well. - A machine has 4 physical CPUs - There are 4 guests on the machine - Each guest has 2 virtual CPUs So we've overcommit the host CPU resources x2 here. Lets say that we want to use this feature to ensure consistent top end performance of every guest, splitting the host pCPUs resources evenly across all guests, so each guest is ensured 1 pCPU worth of CPU time overall. This patch lets you do this by assigning caps per VCPU. So in this example, each VCPU cgroup would have to be configured to cap the VCPUs at 50% of a single pCPU. This leaves the other QEMU threads uncapped / unaccounted for. If any one guest causes non-trivial compute load in a non-VCPU thread, this can/will impact the top-end compute performance of all the other guests on the machine. If we did caps per VM, then you could set the VM cgroup such that the VM as a whole had 100% of a single pCPU. If a guest is 100% compute bound, it can use its full 100% of a pCPU allocation in vCPU threads. If any other guest is causing CPU time in a non-VCPU thread, it cannot impact the top end compute performance of VCPU threads in the other guests. A per-VM cap would, however, mean a guest with 2 vCPUs could have unequal scheduling, where one vCPU claimed 75% of the pCPU and the othe vCPU got left with only 25%. So AFAICT, per-VM cgroups is better for ensuring top end compute performance of a guest as a whole, but per-VCPU cgroups can ensure consistent top end performance across vCPUs within a guest. IMHO, per-VM cgroups is the more useful because it is the only way to stop guests impacting each other, but there could be additional benefits of *also* have per-VCPU cgroups if you want to ensure fairness of top-end performance across vCPUs inside a single VM. What this says to me is that per-VM cgroups _in_addition_to_ per-vcpu cgroups is the _most_ useful situation. Since I can't think of any cases where someone would want per-vm and not per-vcpu, how about we always do both when supported. We can still use one pair of tunables (period and quota) and try to do the right thing. For example: vcpus2/vcpus cputune period50/period quota25/quota /cputune Would have the following behavior for qemu-kvm (vcpu threads) Global VM cgroup: cfs_period:50 cfs_quota:50 Each vcpu cgroup: cfs_period:50 cfs_quota:25 and this behavior for qemu with no vcpu threads So, whatever quota value is in the XML, you would multiply that by the number of vCPUS and use it to set the VM quota value ? Yep. I'm trying to think if there is ever a case where you don't want the VM to be a plain multiple of the VCPU value, but I can't think of one. So only the real discussion point here, is whether the quota value in the XML, is treated as a per-VM value, or a per-VCPU value. I think it has to be per-VCPU. Otherwise the user will have to remember to do the multiplication themselves. If they forget to do this they will get a nasty performance surprise. cpu_shares is treated as a per-VM value, period doesn't matter but cpu_quota would be a per-VCPU value, multiplied to get a per-VM value when needed. I still find this mis-match rather wierd to be fair. Yes, this is unfortunate. But cpu_shares is a comparative value whereas quota is quantitative. In the future we could apply 'shares' at the vcpu level too. In that case we'd just pick some arbitrary number and apply it to each vcpu cgroup. So current behaviour if vCPU threads set quota in vCPU group else set nVCPUs * quota in VM group Would change to set nVCPUs * quota in VM group if vCPU threads set quota in vCPU group ? Yes. We need to remember to update the VM cgroup if we change the number of vCPUs on a running guest of course When can this happen? Does libvirt support cpu hotplug? -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir
[libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
Here are the patches to implement the BlockPull/BlockJob API as discussed and agreed to. I am testing with a python script (included for completeness as the final patch). The qemu monitor interface is not expected to change in the future. Stefan is planning to submit placeholder commands for upstream qemu until the generic streaming support is implemented. Changes since V1: - Make virDomainBlockPullAbort() and virDomainGetBlockPullInfo() into a generic BlockJob interface. - Added virDomainBlockJobSetSpeed() - Rename VIR_DOMAIN_EVENT_ID_BLOCK_PULL event to fit into block job API - Add bandwidth argument to virDomainBlockPull() Summary of changes since first generation patch series: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names -- To help speed the provisioning process for large domains, new QED disks are created with backing to a template image. These disks are configured with copy on read such that blocks that are read from the backing file are copied to the new disk. This reduces I/O over a potentially costly path to the backing image. In such a configuration, there is a desire to remove the dependency on the backing image as the domain runs. To accomplish this, qemu will provide an interface to perform sequential copy on read operations during normal VM operation. Once all data has been copied, the disk image's link to the backing file is removed. The virDomainBlockPull API family brings this functionality to libvirt. virDomainBlockPull() instructs the hypervisor to stream the entire device in the background. Progress of this operation can be checked with the function virDomainBlockJobInfo(). An ongoing stream can be cancelled with virDomainBlockJobAbort(). virDomainBlockJobSetSpeed() allows you to limit the bandwidth that the operation may consume. An event (VIR_DOMAIN_EVENT_ID_BLOCK_JOB) will be emitted when a disk has been fully populated or if a BlockPull() operation was terminated due to an error. This event is useful to avoid polling on virDomainBlockJobInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. make check: PASS make syntax-check: PASS make -C tests valgrind: PASS [PATCH 1/8] Add new API virDomainBlockPull* to headers [PATCH 2/8] virDomainBlockPull: Implement the main entry points [PATCH 3/8] Add virDomainBlockPull support to the remote driver [PATCH 4/8] Implement virDomainBlockPull for the qemu driver [PATCH 5/8] Enable the virDomainBlockPull API in virsh [PATCH 6/8] Enable virDomainBlockPull in the python API. [PATCH 7/8] Asynchronous event for BlockJob completion [PATCH 8/8] Test the blockJob/BlockPull API -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] Enable the virDomainBlockPull API in virsh
Define two new virsh commands:
* blockpull: Initiate a blockPull for the given disk
* blockjob: Retrieve progress info, modify speed, and cancel active block jobs
Share print_job_progress() with the migration code.
* tools/virsh.c: implement the new commands
Signed-off-by: Adam Litke a...@us.ibm.com
---
tools/virsh.c | 135 +++--
1 files changed, 131 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index a6803d8..2b590d3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4527,7 +4527,8 @@ out_sig:
}
static void
-print_job_progress(unsigned long long remaining, unsigned long long total)
+print_job_progress(const char *label, unsigned long long remaining,
+ unsigned long long total)
{
int progress;
@@ -4547,7 +4548,7 @@ print_job_progress(unsigned long long remaining, unsigned
long long total)
}
}
-fprintf(stderr, \rMigration: [%3d %%], progress);
+fprintf(stderr, \r%s: [%3d %%], label, progress);
}
static bool
@@ -4632,7 +4633,7 @@ repoll:
functionReturn = true;
if (verbose) {
/* print [100 %] */
-print_job_progress(0, 1);
+print_job_progress(Migration, 0, 1);
}
} else
functionReturn = false;
@@ -4668,7 +4669,8 @@ repoll:
ret = virDomainGetJobInfo(dom, jobinfo);
pthread_sigmask(SIG_SETMASK, oldsigmask, NULL);
if (ret == 0)
-print_job_progress(jobinfo.dataRemaining, jobinfo.dataTotal);
+print_job_progress(Migration, jobinfo.dataRemaining,
+ jobinfo.dataTotal);
}
}
@@ -4771,6 +4773,129 @@ done:
return ret;
}
+typedef enum {
+VSH_CMD_BLOCK_JOB_ABORT = 0,
+VSH_CMD_BLOCK_JOB_INFO = 1,
+VSH_CMD_BLOCK_JOB_SPEED = 2,
+VSH_CMD_BLOCK_JOB_PULL = 3,
+} VSH_CMD_BLOCK_JOB_MODE;
+
+static int
+blockJobImpl(vshControl *ctl, const vshCmd *cmd,
+ virDomainBlockJobInfoPtr info, int mode)
+{
+virDomainPtr dom = NULL;
+const char *name, *path;
+unsigned long bandwidth = 0;
+int ret = -1;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+goto out;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
+goto out;
+
+if (vshCommandOptString(cmd, path, path) 0)
+goto out;
+
+if (vshCommandOptUL(cmd, bandwidth, bandwidth) 0)
+goto out;
+
+if (mode == VSH_CMD_BLOCK_JOB_ABORT)
+ret = virDomainBlockJobAbort(dom, path, 0);
+else if (mode == VSH_CMD_BLOCK_JOB_INFO)
+ret = virDomainGetBlockJobInfo(dom, path, info, 0);
+else if (mode == VSH_CMD_BLOCK_JOB_SPEED)
+ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0);
+else if (mode == VSH_CMD_BLOCK_JOB_PULL)
+ret = virDomainBlockPull(dom, path, bandwidth, 0);
+
+out:
+virDomainFree(dom);
+return ret;
+}
+
+/*
+ * blockpull command
+ */
+static const vshCmdInfo info_block_pull[] = {
+{help, N_(Populate a disk from its backing image.)},
+{desc, N_(Populate a disk from its backing image.)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_block_pull[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{path, VSH_OT_DATA, VSH_OFLAG_REQ, N_(Fully-qualified path of disk)},
+{bandwidth, VSH_OT_DATA, VSH_OFLAG_NONE, N_(Bandwidth limit in MB/s)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
+{
+if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_PULL) != 0)
+return false;
+return true;
+}
+
+/*
+ * blockjobinfo command
+ */
+static const vshCmdInfo info_block_job[] = {
+{help, N_(Manage active block operations.)},
+{desc, N_(Manage active block operations.)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_block_job[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{path, VSH_OT_DATA, VSH_OFLAG_REQ, N_(Fully-qualified path of disk)},
+{abort, VSH_OT_BOOL, VSH_OFLAG_NONE, N_(Abort the active job on the
speficied disk)},
+{info, VSH_OT_BOOL, VSH_OFLAG_NONE, N_(Get active job information for
the specified disk)},
+{bandwidth, VSH_OT_DATA, VSH_OFLAG_NONE, N_(Set the Bandwidth limit in
MB/s)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
+{
+int mode;
+virDomainBlockJobInfo info;
+const char *type;
+int ret;
+
+if (vshCommandOptBool (cmd, abort)) {
+mode = VSH_CMD_BLOCK_JOB_ABORT;
+} else if (vshCommandOptBool (cmd, info)) {
+mode = VSH_CMD_BLOCK_JOB_INFO;
+} else if (vshCommandOptBool (cmd, bandwidth)) {
+mode = VSH_CMD_BLOCK_JOB_SPEED;
+} else {
+vshError(ctl, %s,
+ _(One of --abort, --info, or --bandwidth is required
[libvirt] [PATCH 1/8] Add new API virDomainBlockPull* to headers
Set up the types for the block pull functions and insert them into the
virDriver structure definition. Symbols are exported in this patch to prevent
documentation compile failures.
* include/libvirt/libvirt.h.in: new API
* src/driver.h: add the new entry to the driver structure
* python/generator.py: fix compiler errors, the actual python bindings are
implemented later
* src/libvirt_public.syms: export symbols
* docs/apibuild.py: Extend 'unsigned long' parameter exception to this API
Signed-off-by: Adam Litke a...@us.ibm.com
---
docs/apibuild.py |7 -
include/libvirt/libvirt.h.in | 44 ++
python/generator.py |2 +
src/driver.h | 24 ++
src/libvirt_public.syms |7 ++
5 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/docs/apibuild.py b/docs/apibuild.py
index 6e35cfb..53b3421 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -1641,7 +1641,9 @@ class CParser:
virDomainMigrateSetMaxSpeed: (False, (bandwidth)),
virDomainSetMaxMemory : (False, (memory)),
virDomainSetMemory : (False, (memory)),
-virDomainSetMemoryFlags: (False, (memory)) }
+virDomainSetMemoryFlags: (False, (memory)),
+virDomainBlockJobSetSpeed : (False, (bandwidth)),
+virDomainBlockPull : (False, (bandwidth)) }
def checkLongLegacyFunction(self, name, return_type, signature):
if long in return_type and long long not in return_type:
@@ -1667,7 +1669,8 @@ class CParser:
# [unsigned] long long
long_legacy_struct_fields = \
{ _virDomainInfo : (maxMem, memory),
-_virNodeInfo : (memory) }
+_virNodeInfo : (memory),
+_virDomainBlockJobInfo : (bandwidth) }
def checkLongLegacyStruct(self, name, fields):
for field in fields:
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 607b5bc..23947c7 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1375,6 +1375,50 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);
/*
+ * BlockJob API
+ */
+
+/**
+ * virDomainBlockJobType:
+ *
+ * VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: Block Pull (virDomainBlockPull)
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0,
+VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
+} virDomainBlockJobType;
+
+/* An iterator for monitoring block job operations */
+typedef unsigned long long virDomainBlockJobCursor;
+
+typedef struct _virDomainBlockJobInfo virDomainBlockJobInfo;
+struct _virDomainBlockJobInfo {
+virDomainBlockJobType type;
+unsigned long bandwidth;
+/*
+ * The following fields provide an indication of block job progress. @cur
+ * indicates the current position and will be between 0 and @end. @end is
+ * the final cursor position for this operation and represents completion.
+ * To approximate progress, divide @cur by @end.
+ */
+virDomainBlockJobCursor cur;
+virDomainBlockJobCursor end;
+};
+typedef virDomainBlockJobInfo *virDomainBlockJobInfoPtr;
+
+int virDomainBlockJobAbort(virDomainPtr dom, const char *path,
+ unsigned int flags);
+int virDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
+ virDomainBlockJobInfoPtr info,
+ unsigned int flags);
+intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
+ unsigned long bandwidth, unsigned int flags);
+
+int virDomainBlockPull(virDomainPtr dom, const char *path,
+ unsigned long bandwidth, unsigned int flags);
+
+
+/*
* NUMA support
*/
diff --git a/python/generator.py b/python/generator.py
index 1cb82f5..b25c74e 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -186,6 +186,7 @@ def enum(type, name, value):
functions_failed = []
functions_skipped = [
virConnectListDomains,
+'virDomainGetBlockJobInfo',
]
skipped_modules = {
@@ -202,6 +203,7 @@ skipped_types = {
'virStreamEventCallback': No function types in python,
'virEventHandleCallback': No function types in python,
'virEventTimeoutCallback': No function types in python,
+ 'virDomainBlockJobInfoPtr': Not implemented yet,
}
###
diff --git a/src/driver.h b/src/driver.h
index 9d0d3de..776bb7f 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -661,6 +661,26 @@ typedef int
unsigned long flags,
int cancelled);
+
+typedef int
+(*virDrvDomainBlockJobAbort)(virDomainPtr dom, const char *path,
+ unsigned int flags);
+
+typedef
[libvirt] [PATCH 4/8] Implement virDomainBlockPull for the qemu driver
The virDomainBlockPull* family of commands are enabled by the
following HMP/QMP commands: 'block_stream', 'block_job_cancel',
'info block-jobs' / 'query-block-jobs', and 'block_job_set_speed'.
* src/qemu/qemu_driver.c src/qemu/qemu_monitor_text.[ch]: implement disk
streaming by using the proper qemu monitor commands.
* src/qemu/qemu_monitor_json.[ch]: implement commands using the qmp monitor
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/qemu/qemu_driver.c | 113 +
src/qemu/qemu_monitor.c | 18 +
src/qemu/qemu_monitor.h | 13
src/qemu/qemu_monitor_json.c | 147 ++
src/qemu/qemu_monitor_json.h |5 ++
src/qemu/qemu_monitor_text.c | 162 ++
src/qemu/qemu_monitor_text.h |6 ++
7 files changed, 464 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8870e33..0f556a9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8493,6 +8493,115 @@ cleanup:
return ret;
}
+static const char *
+qemuDiskPathToAlias(virDomainObjPtr vm, const char *path) {
+int i;
+char *ret = NULL;
+
+for (i = 0 ; i vm-def-ndisks ; i++) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+
+if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK
+disk-type != VIR_DOMAIN_DISK_TYPE_FILE)
+continue;
+
+if (disk-src != NULL STREQ(disk-src, path)) {
+if (virAsprintf(ret, drive-%s, disk-info.alias) 0) {
+virReportOOMError();
+return NULL;
+}
+break;
+}
+}
+
+if (!ret) {
+qemuReportError(VIR_ERR_INVALID_ARG,
+%s, _(No device found for specified path));
+}
+return ret;
+}
+
+static int
+qemuDomainBlockJobImpl(virDomainPtr dom, const char *path,
+ unsigned long bandwidth, virDomainBlockJobInfoPtr info,
+ int mode)
+{
+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, path);
+if (!device) {
+goto cleanup;
+}
+
+if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm));
+priv = vm-privateData;
+ret = qemuMonitorBlockJob(priv-mon, device, bandwidth, info, mode);
+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 int
+qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int flags)
+{
+virCheckFlags(0, -1);
+return qemuDomainBlockJobImpl(dom, path, 0, NULL, BLOCK_JOB_ABORT);
+}
+
+static int
+qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
+ virDomainBlockJobInfoPtr info, unsigned int flags)
+{
+virCheckFlags(0, -1);
+return qemuDomainBlockJobImpl(dom, path, 0, info, BLOCK_JOB_INFO);
+}
+
+static int
+qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
+ unsigned long bandwidth, unsigned int flags)
+{
+virCheckFlags(0, -1);
+return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_SPEED);
+}
+
+static int
+qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long
bandwidth,
+unsigned int flags)
+{
+virCheckFlags(0, -1);
+return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_PULL);
+}
static virDriver qemuDriver = {
.no = VIR_DRV_QEMU,
@@ -8619,6 +8728,10 @@ static virDriver qemuDriver = {
.domainMigratePerform3 = qemuDomainMigratePerform3, /* 0.9.2 */
.domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */
.domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */
+.domainBlockJobAbort = qemuDomainBlockJobAbort, /* 0.9.4 */
+.domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */
+.domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
+.domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
};
diff --git a/src/qemu/qemu_monitor.c b/src/qemu
[libvirt] [PATCH 7/8] Asynchronous event for BlockJob completion
When an operation started by virDomainBlockPull completes (either with
success or with failure), raise an event to indicate the final status. This
allows an API user to avoid polling on virDomainGetBlockJobInfo if they would
prefer to use the event mechanism.
* daemon/remote.c: Dispatch events to client
* include/libvirt/libvirt.h.in: Define event ID and callback signature
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Extend API to handle the new event
* src/qemu/qemu_driver.c: Connect to the QEMU monitor event
for block_stream completion and emit a libvirt block pull event
* src/remote/remote_driver.c: Receive and dispatch events to application
* src/remote/remote_protocol.x: Wire protocol definition for the event
* src/remote_protocol-structs: structure definitions for protocol verification
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
src/qemu/qemu_monitor_json.c: Watch for BLOCK_STREAM_COMPLETED event
from QEMU monitor
Signed-off-by: Adam Litke a...@us.ibm.com
---
daemon/remote.c | 31 ++
include/libvirt/libvirt.h.in | 29 +
python/libvirt-override-virConnect.py | 12 +++
python/libvirt-override.c | 52 ++
src/conf/domain_event.c | 56 +
src/conf/domain_event.h |9 +-
src/libvirt_private.syms |2 +
src/qemu/qemu_monitor.c | 13 +++
src/qemu/qemu_monitor.h | 10 ++
src/qemu/qemu_monitor_json.c | 40 +++
src/qemu/qemu_process.c | 31 ++
src/remote/remote_driver.c| 31 ++
src/remote/remote_protocol.x | 10 +-
src/remote_protocol-structs |8 -
14 files changed, 331 insertions(+), 3 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index b471abc..939044c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -339,6 +339,36 @@ static int remoteRelayDomainEventGraphics(virConnectPtr
conn ATTRIBUTE_UNUSED,
return 0;
}
+static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ const char *path,
+ int type,
+ int status,
+ void *opaque)
+{
+virNetServerClientPtr client = opaque;
+remote_domain_event_block_job_msg data;
+
+if (!client)
+return -1;
+
+VIR_DEBUG(Relaying domain block job event %s %d %s %i, %i,
+ dom-name, dom-id, path, type, status);
+
+/* build return data */
+memset(data, 0, sizeof data);
+make_nonnull_domain(data.dom, dom);
+data.path = (char*)path;
+data.type = type;
+data.status = status;
+
+remoteDispatchDomainEventSend(client, remoteProgram,
+ REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB,
+
(xdrproc_t)xdr_remote_domain_event_block_job_msg, data);
+
+return 0;
+}
+
static int remoteRelayDomainEventControlError(virConnectPtr conn
ATTRIBUTE_UNUSED,
virDomainPtr dom,
@@ -373,6 +403,7 @@ static virConnectDomainEventGenericCallback
domainEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventGraphics),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventIOErrorReason),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventControlError),
+VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob),
};
verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 23947c7..d215655 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2735,6 +2735,34 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
void *opaque);
/**
+ * virConnectDomainEventBlockJobStatus:
+ *
+ * The final status of a virDomainBlockPullAll() operation
+ */
+typedef enum {
+VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
+VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
+} virConnectDomainEventBlockJobStatus;
+
+/**
+ * virConnectDomainEventBlockJobCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @path: fully-qualified filename of the affected disk
+ * @type: type of block job (virDomainBlockJobType)
+ * @status: final status of the operation (virConnectDomainEventBlockJobStatus)
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_DOMAIN_EVENT_ID_BLOCK_JOB with virConnectDomainEventRegisterAny()
+ */
+typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn
[libvirt] [PATCH 2/8] virDomainBlockPull: Implement the main entry points
* src/libvirt.c: implement the main entry points
Signed-off-by: Adam Litke a...@us.ibm.com
---
src/libvirt.c | 226 +
1 files changed, 226 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 39e2041..5ca8c03 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -15417,3 +15417,229 @@ error:
virDispatchError(conn);
return -1;
}
+
+/**
+ * virDomainBlockJobAbort:
+ * @dom: pointer to domain object
+ * @path: fully-qualified filename of disk
+ * @flags: currently unused, for future extension
+ *
+ * Cancel the active block job on the given disk.
+ *
+ * Returns -1 in case of failure, 0 when successful.
+ */
+int virDomainBlockJobAbort(virDomainPtr dom, const char *path,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(dom, path=%p, flags=%x, path, 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 (!path) {
+virLibDomainError(VIR_ERR_INVALID_ARG,
+ _(path is NULL));
+goto error;
+}
+
+if (conn-driver-domainBlockJobAbort) {
+int ret;
+ret = conn-driver-domainBlockJobAbort(dom, path, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(dom-conn);
+return -1;
+}
+
+/**
+ * virDomainGetBlockJobInfo:
+ * @dom: pointer to domain object
+ * @path: fully-qualified filename of disk
+ * @info: pointer to a virDomainBlockJobInfo structure
+ * @flags: currently unused, for future extension
+ *
+ * Request block job information for the given disk. If an operation is active
+ * @info will be updated with the current progress.
+ *
+ * Returns -1 in case of failure, 0 when nothing found, 1 when info was found.
+ */
+int virDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
+ virDomainBlockJobInfoPtr info, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(dom, path=%p, info=%p, flags=%x, path, info, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
+virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+conn = dom-conn;
+
+if (!path) {
+virLibDomainError(VIR_ERR_INVALID_ARG,
+ _(path is NULL));
+goto error;
+}
+
+if (!info) {
+virLibDomainError(VIR_ERR_INVALID_ARG,
+ _(info is NULL));
+goto error;
+}
+
+if (conn-driver-domainGetBlockJobInfo) {
+int ret;
+ret = conn-driver-domainGetBlockJobInfo(dom, path, info, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(dom-conn);
+return -1;
+}
+
+/**
+ * virDomainBlockJobSetSpeed:
+ * @dom: pointer to domain object
+ * @path: fully-qualified filename of disk
+ * @bandwidth: specify bandwidth limit in Mbps
+ * @flags: currently unused, for future extension
+ *
+ * Set the maximimum allowable bandwidth that a block job may consume. If
+ * bandwidth is 0, the limit will revert to the hypervisor default.
+ *
+ * Returns -1 in case of failure, 0 when successful.
+ */
+int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
+ unsigned long bandwidth, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(dom, path=%p, bandwidth=%lu, flags=%x,
+ path, bandwidth, 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 (!path) {
+virLibDomainError(VIR_ERR_INVALID_ARG,
+ _(path is NULL));
+goto error;
+}
+
+if (conn-driver-domainBlockJobSetSpeed) {
+int ret;
+ret = conn-driver-domainBlockJobSetSpeed(dom, path, bandwidth,
flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(dom-conn);
+return -1;
+}
+
+/**
+ * virDomainBlockPull:
+ * @dom: pointer to domain object
+ * @path: Fully-qualified filename of disk
+ * @bandwidth: (optional) specify copy bandwidth
[libvirt] [PATCH 6/8] Enable virDomainBlockPull in the python API.
virDomainGetBlockJobInfo requires manual override since it returns a custom
type.
* python/generator.py: reenable bindings for this entry point
* python/libvirt-override-api.xml python/libvirt-override.c:
manual overrides
Signed-off-by: Adam Litke a...@us.ibm.com
---
python/generator.py |2 +-
python/libvirt-override-api.xml |7 +++
python/libvirt-override.c | 39 +++
3 files changed, 47 insertions(+), 1 deletions(-)
diff --git a/python/generator.py b/python/generator.py
index b25c74e..d0d3ae6 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -186,7 +186,6 @@ def enum(type, name, value):
functions_failed = []
functions_skipped = [
virConnectListDomains,
-'virDomainGetBlockJobInfo',
]
skipped_modules = {
@@ -370,6 +369,7 @@ skip_impl = (
'virDomainSendKey',
'virNodeGetCPUStats',
'virNodeGetMemoryStats',
+'virDomainGetBlockJobInfo',
)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 01207d6..268f897 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -320,5 +320,12 @@
arg name='flags' type='unsigned int' info='flags, curently unused'/
return type='int' info=0 on success, -1 on error/
/function
+function name='virDomainGetBlockJobInfo' file='python'
+ infoGet progress information for a block job/info
+ arg name='dom' type='virDomainPtr' info='pointer to the domain'/
+ arg name='path' type='const char *' info='Fully-qualified filename of
disk'/
+ arg name='flags' type='unsigned int' info='fine-tuning flags, currently
unused, pass 0.'/
+ return type='virDomainBlockJobInfo' info='A dictionary containing job
information.' /
+/function
/symbols
/api
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index b713b6a..e89bc97 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2413,6 +2413,44 @@ libvirt_virDomainGetJobInfo(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
return(py_retval);
}
+static PyObject *
+libvirt_virDomainGetBlockJobInfo(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+virDomainPtr domain;
+PyObject *pyobj_domain;
+const char *path;
+unsigned int flags;
+virDomainBlockJobInfo info;
+int c_ret;
+PyObject *ret;
+
+if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainGetBlockJobInfo,
+ pyobj_domain, path, flags))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_ret = virDomainGetBlockJobInfo(domain, path, info, 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(type),
+ libvirt_intWrap(info.type));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(bandwidth),
+ libvirt_ulongWrap(info.bandwidth));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(cur),
+ libvirt_ulonglongWrap(info.cur));
+PyDict_SetItem(ret, libvirt_constcharPtrWrap(end),
+ libvirt_ulonglongWrap(info.end));
+
+return ret;
+}
/***
* Helper functions to avoid importing modules
@@ -3872,6 +3910,7 @@ static PyMethodDef libvirtMethods[] = {
{(char *) virDomainGetJobInfo, libvirt_virDomainGetJobInfo,
METH_VARARGS, NULL},
{(char *) virDomainSnapshotListNames,
libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL},
{(char *) virDomainRevertToSnapshot, libvirt_virDomainRevertToSnapshot,
METH_VARARGS, NULL},
+{(char *) virDomainGetBlockJobInfo, libvirt_virDomainGetBlockJobInfo,
METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
};
--
1.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] Add virDomainBlockPull support to the remote driver
The generator can handle everything except virDomainGetBlockJobInfo().
* src/remote/remote_protocol.x: provide defines for the new entry points
* src/remote/remote_driver.c daemon/remote.c: implement the client and
server side for virDomainGetBlockJobInfo.
* src/remote_protocol-structs: structure definitions for protocol verification
* src/rpc/gendispatch.pl: Permit some unsigned long parameters
Signed-off-by: Adam Litke a...@us.ibm.com
---
daemon/remote.c | 42 ++
src/remote/remote_driver.c | 42 ++
src/remote/remote_protocol.x | 41 -
src/remote_protocol-structs | 34 ++
src/rpc/gendispatch.pl |2 ++
5 files changed, 160 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index daad39d..b471abc 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1587,6 +1587,48 @@ no_memory:
goto cleanup;
}
+static int
+remoteDispatchDomainGetBlockJobInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+virNetMessageHeaderPtr hdr
ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_get_block_job_info_args
*args,
+remote_domain_get_block_job_info_ret *ret)
+{
+virDomainPtr dom = NULL;
+virDomainBlockJobInfo 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;
+
+rv = virDomainGetBlockJobInfo(dom, args-path, tmp, args-flags);
+if (rv = 0)
+goto cleanup;
+
+ret-type = tmp.type;
+ret-bandwidth = tmp.bandwidth;
+ret-cur = tmp.cur;
+ret-end = tmp.end;
+ret-found = 1;
+rv = 0;
+
+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 c2f8bbd..a70b455 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1995,6 +1995,44 @@ done:
return rv;
}
+static int remoteDomainGetBlockJobInfo(virDomainPtr domain,
+ const char *path,
+ virDomainBlockJobInfoPtr info,
+ unsigned int flags)
+{
+int rv = -1;
+remote_domain_get_block_job_info_args args;
+remote_domain_get_block_job_info_ret ret;
+struct private_data *priv = domain-conn-privateData;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(args.dom, domain);
+args.path = (char *)path;
+args.flags = flags;
+
+if (call(domain-conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_JOB_INFO,
+ (xdrproc_t)xdr_remote_domain_get_block_job_info_args,
+ (char *)args,
+ (xdrproc_t)xdr_remote_domain_get_block_job_info_ret,
+ (char *)ret) == -1)
+goto done;
+
+if (ret.found) {
+info-type = ret.type;
+info-bandwidth = ret.bandwidth;
+info-cur = ret.cur;
+info-end = ret.end;
+rv = 1;
+} else {
+rv = 0;
+}
+
+done:
+remoteDriverUnlock(priv);
+return rv;
+}
+
/*--*/
static virDrvOpenStatus ATTRIBUTE_NONNULL (1)
@@ -4254,6 +4292,10 @@ static virDriver remote_driver = {
.domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */
.domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */
.domainSendKey = remoteDomainSendKey, /* 0.9.3 */
+.domainBlockJobAbort = remoteDomainBlockJobAbort, /* 0.9.4 */
+.domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */
+.domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
+.domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d72a60d..96113d8 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -980,6 +980,40 @@ struct remote_domain_set_autostart_args {
int autostart;
};
+struct remote_domain_block_job_abort_args {
+remote_nonnull_domain dom;
+remote_nonnull_string path;
+unsigned int flags;
+};
+
+struct remote_domain_get_block_job_info_args {
+remote_nonnull_domain dom;
+remote_nonnull_string path;
+unsigned int flags;
+};
+
+struct
[libvirt] [PATCH 8/8] Test the blockJob/BlockPull API
This patch is for information only and should not be comitted.
Signed-off-by: Adam Litke a...@us.ibm.com
---
blockPull-test.py | 281 +
1 files changed, 281 insertions(+), 0 deletions(-)
create mode 100644 blockPull-test.py
diff --git a/blockPull-test.py b/blockPull-test.py
new file mode 100644
index 000..a75e0d9
--- /dev/null
+++ b/blockPull-test.py
@@ -0,0 +1,281 @@
+#!/usr/bin/env python
+
+import sys
+import subprocess
+import time
+import unittest
+import re
+import threading
+import libvirt
+
+qemu_img_bin = /home/aglitke/src/qemu/qemu-img
+virsh_bin = /home/aglitke/src/libvirt/tools/virsh
+
+dom_xml =
+domain type='kvm'
+ nameblockPull-test/name
+ memory131072/memory
+ currentMemory131072/currentMemory
+ vcpu1/vcpu
+ os
+type arch='x86_64' machine='pc-0.13'hvm/type
+boot dev='hd'/
+ /os
+ features
+acpi/
+apic/
+pae/
+ /features
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashrestart/on_crash
+ devices
+
emulator/home/aglitke/src/qemu/x86_64-softmmu/qemu-system-x86_64/emulator
+disk type='file' device='disk'
+ driver name='qemu' type='qed'/
+ source file='/tmp/disk1.qed' /
+ target dev='vda' bus='virtio'/
+/disk
+disk type='file' device='disk'
+ driver name='qemu' type='qed'/
+ source file='/tmp/disk2.qed' /
+ target dev='vdb' bus='virtio'/
+/disk
+disk type='file' device='disk'
+ driver name='qemu' type='raw'/
+ source file='/tmp/disk3.raw' /
+ target dev='vdc' bus='virtio'/
+/disk
+graphics type='vnc' port='-1' autoport='yes'/
+ /devices
+/domain
+
+
+def qemu_img(*args):
+global qemu_img_bin
+
+devnull = open('/dev/null', 'r+')
+return subprocess.call([qemu_img_bin] + list(args), stdin=devnull,
stdout=devnull)
+
+def virsh(*args):
+global virsh_bin
+
+devnull = open('/dev/null', 'r+')
+return subprocess.Popen([virsh_bin] + list(args),
+stdout=subprocess.PIPE).communicate()[0]
+#return subprocess.call([virsh_bin] + list(args),
+# stdin=devnull, stdout=devnull, stderr=devnull)
+
+def make_baseimage(name, size_mb):
+devnull = open('/dev/null', 'r+')
+return subprocess.call(['dd', 'if=/dev/zero', of=%s % name, 'bs=1M',
+'count=%i' % size_mb], stdin=devnull,
stdout=devnull, stderr=devnull)
+
+def has_backing_file(path):
+global qemu_img_bin
+p1 = subprocess.Popen([qemu_img_bin, info, path],
+ stdout=subprocess.PIPE).communicate()[0]
+matches = re.findall(^backing file:, p1, re.M)
+if len(matches) 0:
+return True
+return False
+
+class BlockPullTestCase(unittest.TestCase):
+def _error_handler(self, ctx, error, dummy=None):
+pass
+
+def create_disks(self, sparse):
+self.disks = [ '/tmp/disk1.qed', '/tmp/disk2.qed', '/tmp/disk3.raw' ]
+if sparse:
+qemu_img('create', '-f', 'raw', '/tmp/backing1.img', '100M')
+qemu_img('create', '-f', 'raw', '/tmp/backing2.img', '100M')
+else:
+make_baseimage('/tmp/backing1.img', 100)
+make_baseimage('/tmp/backing2.img', 100)
+qemu_img('create', '-f', 'qed', '-o',
'backing_file=/tmp/backing1.img', self.disks[0])
+qemu_img('create', '-f', 'qed', '-o',
'backing_file=/tmp/backing2.img', self.disks[1])
+qemu_img('create', '-f', 'raw', self.disks[2], '100M')
+
+def begin(self, sparse=True):
+global dom_xml
+
+libvirt.registerErrorHandler(self._error_handler, None)
+self.create_disks(sparse)
+self.conn = libvirt.open('qemu:///system')
+self.dom = self.conn.createXML(dom_xml, 0)
+
+def end(self):
+self.dom.destroy()
+self.conn.close()
+
+class TestBasicErrors(BlockPullTestCase):
+def setUp(self):
+self.begin()
+
+def tearDown(self):
+self.end()
+
+def test_bad_path(self):
+try:
+self.dom.blockPull('/dev/null', 0, 0)
+except libvirt.libvirtError, e:
+self.assertEqual(libvirt.VIR_ERR_INVALID_ARG, e.get_error_code())
+else:
+e = self.conn.virConnGetLastError()
+self.assertEqual(libvirt.VIR_ERR_INVALID_ARG, e[0])
+
+def test_abort_no_stream(self):
+try:
+self.dom.blockJobAbort(self.disks[0], 0)
+except libvirt.libvirtError, e:
+self.assertEqual(libvirt.VIR_ERR_OPERATION_INVALID,
e.get_error_code())
+else:
+e = self.conn.virConnGetLastError()
+self.assertEqual(libvirt.VIR_ERR_OPERATION_INVALID, e[0])
+
+def test_start_same_twice(self):
+self.dom.blockPull(self.disks[0], 0, 0)
+try:
+self.dom.blockPull(self.disks[0], 0, 0)
+except libvirt.libvirtError, e:
+self.assertEqual
Re: [libvirt] [PATCH v3 4/6] qemu: Implement period and quota tunable XML configuration and parsing
On 07/18/2011 07:46 PM, Wen Congyang wrote:
At 07/19/2011 04:35 AM, Adam Litke Write:
This is looking good to me. I am pleased to see that the global case is
handled as expected when per-vcpu threads are not active.
On 07/18/2011 04:42 AM, Wen Congyang wrote:
snip
+int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
+{
+virCgroupPtr cgroup = NULL;
+virCgroupPtr cgroup_vcpu = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int rc;
+unsigned int i;
+unsigned long long period = vm-def-cputune.period;
+long long quota = vm-def-cputune.quota;
+
+if (driver-cgroup == NULL)
+return 0; /* Not supported, so claim success */
I just want to check: Is the above logic still valid? Failure to apply
This logic is the same as the logic in the similar function qemuSetupCgroup().
Yes, I am aware. However, the introduction of cpu quotas is a major
feature that defines the amount of computing capacity a domain has
available. In my opinion, if a domain XML file specifies a quota then
'virsh create' should fail of that quota cannot be set up for any reason.
a quota setting (0) could have fairly substantial implications for
system performance. I am not convinced either way but I do think this
point merits some discussion.
+
+rc = virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to find cgroup for %s),
+ vm-def-name);
+goto cleanup;
+}
+
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* If we does not know VCPU-PID mapping or all vcpu runs in the
same
+ * thread, we can not control each vcpu.
+ */
+if (period || quota) {
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_CPU)) {
+if (qemuSetupCgroupVcpuBW(cgroup, period, quota) 0)
+goto cleanup;
+}
+}
+return 0;
+}
+
+for (i = 0; i priv-nvcpupids; i++) {
+rc = virCgroupForVcpu(cgroup, i, cgroup_vcpu, 1);
+if (rc 0) {
+virReportSystemError(-rc,
+ _(Unable to create vcpu cgroup for
%s(vcpu:
+%d)),
+ vm-def-name, i);
+goto cleanup;
+}
+
+/* move the thread for vcpu to sub dir */
+rc = virCgroupAddTask(cgroup_vcpu, priv-vcpupids[i]);
+if (rc 0) {
+virReportSystemError(-rc,
+ _(unable to add vcpu %d task %d to
cgroup),
+ i, priv-vcpupids[i]);
+goto cleanup;
+}
+
+if (period || quota) {
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_CPU)) {
+if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) 0)
+goto cleanup;
+}
+}
+
+virCgroupFree(cgroup_vcpu);
+}
+
+virCgroupFree(cgroup_vcpu);
+virCgroupFree(cgroup);
+return 0;
+
+cleanup:
+virCgroupFree(cgroup_vcpu);
+if (cgroup) {
+virCgroupRemove(cgroup);
+virCgroupFree(cgroup);
+}
+
+return -1;
+}
+
int qemuRemoveCgroup(struct qemud_driver *driver,
virDomainObjPtr vm,
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/6] qemu: Implement period and quota tunable XML configuration and parsing
On 07/18/2011 07:51 PM, Wen Congyang wrote:
At 07/19/2011 04:59 AM, Adam Litke Write:
On 07/18/2011 04:42 AM, Wen Congyang wrote:
+int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
+{
+virCgroupPtr cgroup = NULL;
+virCgroupPtr cgroup_vcpu = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int rc;
+unsigned int i;
+unsigned long long period = vm-def-cputune.period;
+long long quota = vm-def-cputune.quota;
+
+if (driver-cgroup == NULL)
+return 0; /* Not supported, so claim success */
+
+rc = virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to find cgroup for %s),
+ vm-def-name);
+goto cleanup;
+}
+
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* If we does not know VCPU-PID mapping or all vcpu runs in the
same
+ * thread, we can not control each vcpu.
+ */
+if (period || quota) {
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_CPU)) {
+if (qemuSetupCgroupVcpuBW(cgroup, period, quota) 0)
+goto cleanup;
+}
+}
+return 0;
+}
I found a problem above. In the case where we are controlling quota at
the domain level cgroup we must multiply the user-specified quota by the
number of vcpus in the domain in order to get the same performance as we
would with per-vcpu cgroups. As written, the vm will be essentially
capped at 1 vcpu worth of quota regardless of the number of vcpus. You
will also have to apply this logic in reverse when reporting the
scheduler statistics so that the quota number is a per-vcpu quantity.
When quota is 1000, and per-vcpu thread is not active, we can start
vm successfully. When the per-vcpu thread is active, and the num of
vcpu is more than 1, we can not start vm if we multiply the user-specified
quota. It will confuse the user: sometimes the vm can be started, but
sometimes the vm can not be started with the same configuration.
I am not sure I understand what you mean. When vcpu threads are active,
the patches work correctly. It is only when you disable vcpu threads
that you need to multiply the quota by the number of vcpus (since you
are now applying it globally). A 4 vcpu guest that is started using an
emulator with vcpu threads active will get 4 times the cpu bandwidth as
compared to starting the identical configuration using an emulator
without vcpu threads. This is because you currently apply the same
quota setting to the full process as you were applying to a single vcpu.
I know that what I am asking for is confusing at first. The quota value
in a domain XML may not match up with the value actually written to the
cgroup filesystem. The same applies for the schedinfo API vs. cgroupfs.
However, my suggestion will result in quotas that match user
expectation. For a 4 vcpu guest with 50% cpu quota, it is more logical
to set period=50,quota=25 without having to know if my qemu
supports vcpu threads.
For example, to limit a guests to 50% CPU we would have these settings
(when period == 50):
1 VCPU2 VCPU4 VCPU8 VCPU
VCPU-threads ON 25252525
VCPU-threads OFF 2550 100 200
With VCPU threads on, the value is applied to each VCPU whereas with
VCPU threads off it is applied globally. This will yield roughly
equivalent performance regardless of whether the underlying qemu process
enables vcpu threads.
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC New virDomainBlockPull API family to libvirt
On 07/18/2011 09:35 AM, Stefan Hajnoczi wrote:
On the other hand I suspect that we are missing the mechanism to
control the rate of the transfer in the new API, which is something
which could be implemented in the old incremental mechanism, but not
anymore. So I wonder if the virDomainBlockPull() shouldn't get an
unsigned long bandwidth (to stay coherent with migration APIs)
before the flags. I don't know if the support is ready in QEmu but
I suspect that will be an important point, so if not present will
be added :-)
Hopefully Stefan can comment on any throttling mechanisms that might be
added to the qemu implementation. It would be nice to have this feature
built into the API to match the migration APIs, but if that is not
feasible then we could always add it later.
If we follow the live migration API then a block_stream_set_speed
command would be used. libvirt would issue it as a separate command
immediately after starting the streaming operation. There is the
window of time after streaming has started but before
block_stream_set_speed has been called where no throttling takes
place, but I don't think this is a practical problem.
It also opens the possibility of allowing the user to change the rate
limit value while the block streaming operation is active.
In light of our decision to provide a generic BlockJobAbort/BlockJobInfo
interface, should SetSpeed be generic too?
virDomainBlockJobSetSpeed() or virDomainBlockPullSetSpeed() ?
The block_stream_set_speed semantics allow the command to be used when
no streaming operation is active. This can be used to set the speed
without a window of time after creation and before set_speed is
called.
If we switch to block_job_set_speed then it is cleaner to restrict the
command to work on active jobs only. This is because there is only
one speed variable kept but there might be multiple job types
(streaming, compaction, etc) which would need different settings.
What do you think about this?
I think the block_job_set_speed semantics seem reasonable to me. What is
the method for querying the current speed setting? I would suggest
extending the query-block-job command to include this information. In
this case, I would extend virDomainBlockJobInfo to contain:
/* The maximum allowable bandwidth for this job (MB/s) */
unsigned long bandwidth;
block_job_set_speed
---
Set maximum speed for a background block operation.
This is a per-block device command that can only be issued
when there is an active block job.
Throttling can be disabled by setting the speed to 0.
Arguments:
- device: device name (json-string)
- value: maximum speed, in bytes per second (json-int)
Errors:
NotSupported: job type does not support speed setting
Example:
- { execute: block_job_set_speed,
arguments: { device: virtio0, value: 1024 } }
Stefan
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v3 3/6] Update XML Schema for new entries
On 07/18/2011 04:41 AM, Wen Congyang wrote: --- docs/schemas/domain.rng | 26 +- 1 files changed, 25 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 8a4e3fe..5f8151d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -388,6 +388,16 @@ ref name=cpushares/ /element /optional + optional +element name=period + ref name=cpuperiod/ +/element + /optional + optional +element name=quota + ref name=cpuquota/ +/element + /optional zeroOrMore element name=vcpupin attribute name=vcpu Yes, this is exactly the interface we are looking for. @@ -2401,7 +2411,21 @@ data type=unsignedInt param name=pattern[0-9]+/param /data - /define + /define + define name=cpuperiod +data type=unsignedLong + param name=pattern[0-9]+/param + param name=minInclusive1000/param + param name=maxInclusive100/param +/data + /define + define name=cpuquota +data type=long + param name=pattern-?[0-9]+/param + param name=maxInclusive18446744073709551/param + param name='minInclusive'-1/param +/data + /define define name=PortNumber data type=short param name=minInclusive-1/param -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v3 5/6] qemu: Implement cfs_period and cfs_quota's modification
On 07/18/2011 04:42 AM, Wen Congyang wrote:
@@ -5983,7 +6169,30 @@ out:
goto cleanup;
}
-*nparams = 1;
+if (*nparams 1) {
+params[1].value.ul = period;
+params[1].type = VIR_TYPED_PARAM_ULLONG;
+if (virStrcpyStatic(params[1].field, cfs_period) == NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s,
+_(Field cfs_period too long for destination));
+goto cleanup;
+}
+
+params[2].value.ul = quota;
Possible buffer overflow if *nparams == 2 ...
+params[2].type = VIR_TYPED_PARAM_LLONG;
+if (virStrcpyStatic(params[2].field, cfs_quota) == NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s,
+_(Field cfs_quota too long for destination));
+goto cleanup;
+}
+
+*nparams = 3;
+} else {
+*nparams = 1;
+}
+
ret = 0;
cleanup:
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/6] qemu: Implement period and quota tunable XML configuration and parsing
On 07/18/2011 04:42 AM, Wen Congyang wrote:
+int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
+{
+virCgroupPtr cgroup = NULL;
+virCgroupPtr cgroup_vcpu = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int rc;
+unsigned int i;
+unsigned long long period = vm-def-cputune.period;
+long long quota = vm-def-cputune.quota;
+
+if (driver-cgroup == NULL)
+return 0; /* Not supported, so claim success */
+
+rc = virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to find cgroup for %s),
+ vm-def-name);
+goto cleanup;
+}
+
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* If we does not know VCPU-PID mapping or all vcpu runs in the
same
+ * thread, we can not control each vcpu.
+ */
+if (period || quota) {
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_CPU)) {
+if (qemuSetupCgroupVcpuBW(cgroup, period, quota) 0)
+goto cleanup;
+}
+}
+return 0;
+}
I found a problem above. In the case where we are controlling quota at
the domain level cgroup we must multiply the user-specified quota by the
number of vcpus in the domain in order to get the same performance as we
would with per-vcpu cgroups. As written, the vm will be essentially
capped at 1 vcpu worth of quota regardless of the number of vcpus. You
will also have to apply this logic in reverse when reporting the
scheduler statistics so that the quota number is a per-vcpu quantity.
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC New virDomainBlockPull API family to libvirt
On 07/15/2011 05:39 AM, Stefan Hajnoczi wrote: On Thu, Jul 14, 2011 at 7:47 PM, Adam Litke a...@us.ibm.com wrote: On 07/13/2011 08:04 PM, Daniel Veillard wrote: On Wed, Jul 13, 2011 at 03:46:30PM -0500, Adam Litke wrote: Unfortunately, after committing the blockPull API to libvirt, the qemu community decided to change the API somewhat and we needed to revert the libvirt implementation. Now that the qemu API is settling out again, I would like to propose an updated libvirt public API which has required only a minor set of changes to sync back up to the qemu API. Summary of changes: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names Okay. snip On the other hand I suspect that we are missing the mechanism to control the rate of the transfer in the new API, which is something which could be implemented in the old incremental mechanism, but not anymore. So I wonder if the virDomainBlockPull() shouldn't get an unsigned long bandwidth (to stay coherent with migration APIs) before the flags. I don't know if the support is ready in QEmu but I suspect that will be an important point, so if not present will be added :-) Hopefully Stefan can comment on any throttling mechanisms that might be added to the qemu implementation. It would be nice to have this feature built into the API to match the migration APIs, but if that is not feasible then we could always add it later. If we follow the live migration API then a block_stream_set_speed command would be used. libvirt would issue it as a separate command immediately after starting the streaming operation. There is the window of time after streaming has started but before block_stream_set_speed has been called where no throttling takes place, but I don't think this is a practical problem. It also opens the possibility of allowing the user to change the rate limit value while the block streaming operation is active. In light of our decision to provide a generic BlockJobAbort/BlockJobInfo interface, should SetSpeed be generic too? virDomainBlockJobSetSpeed() or virDomainBlockPullSetSpeed() ? -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
,
unsigned int flags);
/**
* virConnectDomainEventBlockJobStatus:
*
* The final status of a block job
*/
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
} virConnectDomainEventBlockJobStatus;
/**
* virConnectDomainEventBlockPullCallback:
* @conn: connection object
* @dom: domain on which the event occurred
* @path: fully-qualified filename of the affected disk
* @type: type of block job (virDomainBlockJobType)
* @status: final status of the operation (virConnectDomainEventBlockPullStatus)
* @opaque: callback context
*
* The callback signature to use when registering for an event of type
* VIR_DOMAIN_EVENT_ID_BLOCK_PULL with virConnectDomainEventRegisterAny()
*/
typedef void (*virConnectDomainEventBlockPullCallback)(virConnectPtr conn,
virDomainPtr dom,
const char *path,
int type,
int status,
void *opaque);
/**
* virDomainBlockPull:
* @dom: pointer to domain object
* @path: Fully-qualified filename of disk
* @bandwidth: (optional) specify copy bandwidth limit in Mbps
* @flags: currently unused, for future extension
*
* Populate a disk image with data from its backing image. Once all data from
* its backing image has been pulled, the disk no longer depends on a backing
* image. This function pulls data for the entire device in the background.
* Progress of the operation can be checked with virDomainGetBlockJobInfo() and
* the operation can be aborted with virDomainBlockJobAbort(). When finished,
* an asynchronous event is raised to indicate the final status.
*
* The maximum bandwidth (in Mbps) that will be used to do the copy can be
* specified with the bandwidth parameter. If set to 0, libvirt will choose a
* suitable default. Some hypervisors do not support this feature and will
* return an error if bandwidth is not 0.
*
* Returns 0 if the operation has started, -1 on failure.
*/
int virDomainBlockPull(virDomainPtr dom,
const char *path,
unsigned long bandwidth,
unsigned int flags);
--
Adam Litke
IBM Linux Technology Center
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
