Re: [libvirt] [PATCH] Block info query: Add flag to allow failure if not active
- Original Message - > From: "Daniel P. Berrange" > To: "John Ferlan" > Cc: libvir-list@redhat.com > Sent: Wednesday, December 18, 2013 3:57:22 PM > Subject: Re: [libvirt] [PATCH] Block info query: Add flag to allow failure if > not active > > On Wed, Dec 18, 2013 at 09:49:46AM -0500, John Ferlan wrote: > > > > > > On 12/18/2013 09:35 AM, Daniel P. Berrange wrote: > > > On Wed, Dec 18, 2013 at 06:58:10AM -0500, John Ferlan wrote: > > > > > > > > I'm not convinced this flag is desirable. Can't apps just > > > check whether the guest is running themselves, or indeed > > > something like RHEV surely knows what its own VM is doing > > > already. > > > > I do agree with you and that's been my argument in the referenced BZ; > > however, as I understand it - they have a thread that continually polls > > for blockInfo information and a separate thread that handles the > > migration. The blockInfo thread doesn't have the state information. > > But it must have a virDomainPtr instance, so it can use > virDomainGetState() or virDomainIsActive() if it cares > about this. oVirt is using only transient domains, so it seems to me that the assumption here is that blockInfo should either return a value from a running qemu process or an error (because the domain doesn't exist anymore). Having a short moment where it's treated as a non-transient domain is confusing. It's fine by me if you want to maintain the current implementation for non-transient domains (and therefore the new flag is not needed). > > I contend it's just as simple to add a check about the domain state and > > to get/check the reason value as well. That is - I think the blockInfo > > thread should be more aware of state. Of course, the return argument is > > that libvirt shouldn't return different answers on consecutive fetches > > where the first fetch is when the guest is "active" and the second is > > when it's not. > > Sure, I agree that libvirt isn't ideal here - our hands are tied by > the fact that QEMU doesn't make this data available to us offline. > If we changed anything on the libvirt side, then I'd want to see us > do a proper fix to get the data but that'd require qemu help; I added the support for this to qemu-img. It scans a qcow2 image and reports the highest allocated cluster. Sadly the only way to implement it was on the image check (takes a long time). -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bridge: add the firewalld zone support
This patch adds the support for the firewalld zones in the virtual
bridges managed by libvirt allowing to configure the rules as for
all the other interfaces.
After assigning a fwzone to the bridge it is then possible to
configure the firewall rules through the firwalld daemon, e.g.
enabling or disabling connections to certain services located on
the host.
The firewalld events (e.g. restart, reload) are managed with the
preexisting infrastructure that is already in place to reinstate
the general iptables rules.
Signed-off-by: Federico Simoncelli
---
docs/formatnetwork.html.in | 5 +-
docs/schemas/network.rng| 6 +++
src/conf/network_conf.c | 8 +++-
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 114
5 files changed, 131 insertions(+), 3 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 4dd0415..e6d7d9c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -69,7 +69,7 @@
...
-<bridge name="virbr0" stp="on" delay="5"/>
+<bridge name="virbr0" fwzone="internal" stp="on" delay="5"/>
<domain name="example.com"/>
<forward mode="nat" dev="eth0"/>
...
@@ -91,6 +91,9 @@
is 'on' or 'off' (default is
'on'). Attribute delay sets the bridge's forward
delay value in seconds (default is 0).
+Attribute fwzone specifies the firewalld zone to use
+for the bridge device (the default is no zone and firewalld will
+configure the device with its default zone).
Since 0.3.0
domain
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6c3fae2..2f92cfc 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -66,6 +66,12 @@
+
+
+
+
+
+
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 968cf11..cf163e0 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1749,6 +1749,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
stp = virXPathString("string(./bridge[1]/@stp)", ctxt);
def->stp = (stp && STREQ(stp, "off")) ? 0 : 1;
+def->fwzone = virXPathString("string(./bridge[1]/@fwzone)", ctxt);
if (virXPathULong("string(./bridge[1]/@delay)", ctxt, &def->delay) < 0)
def->delay = 0;
@@ -1880,9 +1881,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
/* fall through to next case */
case VIR_NETWORK_FORWARD_BRIDGE:
-if (def->delay || stp) {
+if (def->delay || stp || def->fwzone) {
virReportError(VIR_ERR_XML_ERROR,
- _("bridge delay/stp options only allowed in route,
nat, and isolated mode, not in %s (network '%s')"),
+ _("bridge delay/stp/fwzone options only allowed in "
+ "route, nat, and isolated mode, not in %s
(network '%s')"),
virNetworkForwardTypeToString(def->forward.type),
def->name);
goto error;
@@ -2370,6 +2372,8 @@ virNetworkDefFormatInternal(virBufferPtr buf,
virBufferAddLit(buf, "bridge)
virBufferEscapeString(buf, " name='%s'", def->bridge);
+if (def->fwzone)
+virBufferEscapeString(buf, " fwzone='%s'", def->fwzone);
virBufferAsprintf(buf, " stp='%s' delay='%ld' />\n",
def->stp ? "on" : "off",
def->delay);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 6ce9a00..80ee75f 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -194,6 +194,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */
char *domain;
+char *fwzone; /* The firewalld zone */
unsigned long delay; /* Bridge forward delay (ms) */
unsigned int stp :1; /* Spanning tree protocol */
virMacAddr mac; /* mac address of bridge device */
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e8b314a..4875bbc 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -76,6 +76,29 @@
#define VIR_FROM_THIS VIR_FROM_NETWORK
+#if HAVE_FIREWALLD
+static char *firewall_cmd_path = NULL;
+
+static int
+networkBridgeOnceInit(void)
+{
+firewall_cmd_path = virFindFi
[libvirt] [PATCH] python: Initialize new_params in virDomainSetSchedulerParameters
The new_params variable must be initialized in case the virDomainGetSchedulerParameters call fails and we hit the cleanup section before actually allocating the new parameters. Signed-off-by: Federico Simoncelli --- python/libvirt-override.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index bb1d881..485ed28 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -700,7 +700,7 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, int i_retval; int nparams = 0; Py_ssize_t size = 0; -virTypedParameterPtr params, new_params; +virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OO:virDomainSetScedulerParameters", &pyobj_domain, &info)) -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for shared sanlock leases
- Original Message -
> From: "Eric Blake"
> To: "Federico Simoncelli"
> Cc: "Daniel P. Berrange" , libvir-list@redhat.com
> Sent: Friday, June 22, 2012 6:19:34 AM
> Subject: Re: [libvirt] [PATCH] Add support for shared sanlock leases
>
> On 06/21/2012 11:00 AM, Federico Simoncelli wrote:
> > On Thu, Jun 21, 2012 at 03:37:10PM +0100, Daniel P. Berrange wrote:
> >> From: "Daniel P. Berrange"
> >>
> >> A sanlock lease can be marked as shared (rather
> >> than exclusive) using SANLK_RES_SHARED flag. This
> >> adds support for that flag and ensures that in auto
> >> disk mode, any shared disks use shared leases. This
> >> also makes any read-only disks be completely
> >> ignored.
> >>
>
> >> case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
> >> if (driver->autoDiskLease) {
> >> -if (virLockManagerSanlockAddDisk(lock, name, nparams,
> >> params) < 0)
> >> +if (virLockManagerSanlockAddDisk(lock, name, nparams,
> >> params,
> >> + !!(flags &
> >> VIR_LOCK_MANAGER_RESOURCE_SHARED)) < 0)
> >
> > Just out of curiosity, you are using "!!" (both here and below)
> > because
> > the compiler is complaining about the type?
>
> Rather, it is a way of forcing a value to be 0 or 1 with the fewest
> characters possible (since the line is already long); shorter than
> constructs such as:
>
> (bool)(flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
> (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED) != 0
Yes, it is clear to me that it was a shortcut to squash the values
to 0|1 (as coincidence Daniel few days ago received a patch from me
with a similar trick). It would be nice to know if the compiler is
smart enough to optimize it (instead of doing the double negation).
Beware... not that it would make any difference here :)
> But since virLockManagerSanlockAddDisk() declared its argument as
> bool, and we already require a C99 compiler, the !! trick is strictly
> unnecessary; C99 compilers already guarantee to implicitly convert
> any non-zero value to true when doing argument conversion.
Ah good, this is what I wanted to know. Is the compiler going to
silently convert the value or is it going to produce a warning?
Looks like you're saying that it's the former.
--
Federico
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for shared sanlock leases
On Thu, Jun 21, 2012 at 03:37:10PM +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> A sanlock lease can be marked as shared (rather
> than exclusive) using SANLK_RES_SHARED flag. This
> adds support for that flag and ensures that in auto
> disk mode, any shared disks use shared leases. This
> also makes any read-only disks be completely
> ignored.
>
> These changes remove the need for the option
>
> ignore_readonly_and_shared_disks
>
> so that is removed
>
> Signed-off-by: Daniel P. Berrange
> ---
> src/locking/lock_driver_sanlock.c | 38
> +++
> src/locking/sanlock.conf|7 --
> src/locking/test_libvirt_sanlock.aug.in |1 -
> 3 files changed, 13 insertions(+), 33 deletions(-)
>
> diff --git a/src/locking/lock_driver_sanlock.c
> b/src/locking/lock_driver_sanlock.c
> index 146aefd..16941c9 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -65,7 +65,6 @@ struct _virLockManagerSanlockDriver {
> bool requireLeaseForDisks;
> int hostID;
> bool autoDiskLease;
> -bool ignoreReadonlyShared;
> char *autoDiskLeasePath;
> };
>
> @@ -115,10 +114,6 @@ static int virLockManagerSanlockLoadConfig(const char
> *configFile)
> CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG);
> if (p) driver->autoDiskLease = p->l;
>
> -p = virConfGetValue(conf, "ignore_readonly_and_shared_disks");
> -CHECK_TYPE("ignore_readonly_and_shared_disks", VIR_CONF_LONG);
> -if (p) driver->ignoreReadonlyShared = p->l;
> -
> p = virConfGetValue(conf, "disk_lease_dir");
> CHECK_TYPE("disk_lease_dir", VIR_CONF_STRING);
> if (p && p->str) {
> @@ -428,7 +423,8 @@ static int virLockManagerSanlockDiskLeaseName(const char
> *path,
> static int virLockManagerSanlockAddLease(virLockManagerPtr lock,
> const char *name,
> size_t nparams,
> - virLockManagerParamPtr params)
> + virLockManagerParamPtr params,
> + bool shared)
> {
> virLockManagerSanlockPrivatePtr priv = lock->privateData;
> int ret = -1;
> @@ -440,6 +436,7 @@ static int
> virLockManagerSanlockAddLease(virLockManagerPtr lock,
> goto cleanup;
> }
>
> +res->flags = shared ? SANLK_RES_SHARED : 0;
> res->num_disks = 1;
> if (!virStrcpy(res->name, name, SANLK_NAME_LEN)) {
> virLockError(VIR_ERR_INTERNAL_ERROR,
> @@ -485,7 +482,8 @@ cleanup:
> static int virLockManagerSanlockAddDisk(virLockManagerPtr lock,
> const char *name,
> size_t nparams,
> -virLockManagerParamPtr params
> ATTRIBUTE_UNUSED)
> +virLockManagerParamPtr params
> ATTRIBUTE_UNUSED,
> +bool shared)
> {
> virLockManagerSanlockPrivatePtr priv = lock->privateData;
> int ret = -1;
> @@ -503,6 +501,7 @@ static int virLockManagerSanlockAddDisk(virLockManagerPtr
> lock,
> goto cleanup;
> }
>
> +res->flags = shared ? SANLK_RES_SHARED : 0;
> res->num_disks = 1;
> if (virLockManagerSanlockDiskLeaseName(name, res->name, SANLK_NAME_LEN)
> < 0)
> goto cleanup;
> @@ -630,27 +629,15 @@ static int
> virLockManagerSanlockAddResource(virLockManagerPtr lock,
> return -1;
> }
>
> -if ((flags & (VIR_LOCK_MANAGER_RESOURCE_READONLY |
> - VIR_LOCK_MANAGER_RESOURCE_SHARED)) &&
> -driver->ignoreReadonlyShared) {
> -return 0;
> -}
> -
> -if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY) {
> -virLockError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("Readonly leases are not supported"));
> -return -1;
> -}
> -if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED) {
> -virLockError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("Shareable leases are not supported"));
> -return -1;
> -}
> +/* Treat R/O resources as a no-op lock request */
> +if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY)
> +return 0;
>
> switch (type) {
> case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
> if (driver->autoDiskLease) {
> -if (virLockManagerSanlockAddDisk(lock, name, nparams, params) <
> 0)
> +if (virLockManagerSanlockAddDisk(lock, name, nparams, params,
> + !!(flags &
> VIR_LOCK_MANAGER_RESOURCE_SHARED)) < 0)
Just out of curiosity, you are using "!!" (both here and below) because
the compiler is complaining about the type?
> return -1;
>
> if
> (virLockManagerSanlockCreateLease(priv->res_args[priv->res_count-1]) < 0)
> @@ -664,
Re: [libvirt] [PATCHv2 08/15] blockjob: expose qemu commands for mirrored storage migration
- Original Message -
> From: "Eric Blake"
> To: libvir-list@redhat.com
> Cc: pbonz...@redhat.com, fsimo...@redhat.com
> Sent: Friday, April 6, 2012 6:36:54 AM
> Subject: [PATCHv2 08/15] blockjob: expose qemu commands for mirrored storage
> migration
>
> The new block copy storage migration sequence requires both the
> 'drive-mirror' action in 'transaction' (present if the 'drive-mirror'
> standalone monitor command also exists) and the 'drive-reopen'
> monitor
> command (it would be nice if that were also part of a 'transaction',
> but the initial qemu implementation has it standalone only).
>
> As of this[1] qemu email, both commands have been proposed but not
> yet
> incorporated into the tree, so there is a risk that qemu 1.1 will
> not have these commands, or will have something subtly different.
> [1]https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg01524.html
>
> * src/qemu/qemu_capabilities.h (QEMU_CAPS_DRIVE_MIRROR)
> (QEMU_CAPS_DRIVE_REOPEN): New bits.
> * src/qemu/qemu_capabilities.c (qemuCaps): Name them.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set
> them.
> (qemuMonitorJSONDriveMirror, qemuMonitorDriveReopen): New functions.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDriveMirror)
> (qemuMonitorDriveReopen): Declare them.
> * src/qemu/qemu_monitor.c (qemuMonitorDriveMirror)
> (qemuMonitorDriveReopen): New passthroughs.
> * src/qemu/qemu_monitor.h (qemuMonitorDriveMirror)
> (qemuMonitorDriveReopen): Declare them.
> ---
> src/qemu/qemu_capabilities.c |3 ++
> src/qemu/qemu_capabilities.h |2 +
> src/qemu/qemu_monitor.c | 50
> src/qemu/qemu_monitor.h | 23 +
> src/qemu/qemu_monitor_json.c | 74
> +++--
> src/qemu/qemu_monitor_json.h | 21 +++-
> 6 files changed, 167 insertions(+), 6 deletions(-)
>
[...]
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index e1a8d4c..f33bed8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2685,6 +2685,32 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon,
> virJSONValuePtr actions,
> return ret;
> }
>
> +/* Add the drive-mirror action to a transaction. */
> +int
> +qemuMonitorDriveMirror(qemuMonitorPtr mon, virJSONValuePtr actions,
> + const char *device, const char *file,
> + const char *format, int mode)
> +{
> +int ret;
> +
> +VIR_DEBUG("mon=%p, actions=%p, device=%s, file=%s, format=%s,
> mode=%o",
> + mon, actions, device, file, format, mode);
> +
> +if (!mon) {
> +qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +_("monitor must not be NULL"));
> +return -1;
> +}
> +
> +if (mon->json)
> +ret = qemuMonitorJSONDriveMirror(mon, actions, device, file,
> format,
> + mode);
> +else
> +qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +_("drive-mirror requires JSON monitor"));
You should set ret to -1 here (or return -1).
> +return ret;
> +}
> +
> /* Use the transaction QMP command to run atomic snapshot commands.
> */
> int
> qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
> @@ -2701,6 +2727,30 @@ qemuMonitorTransaction(qemuMonitorPtr mon,
> virJSONValuePtr actions)
> return ret;
> }
>
> +/* Use the drive-reopen monitor command. */
> +int
> +qemuMonitorDriveReopen(qemuMonitorPtr mon, const char *device,
> + const char *file, const char *format)
> +{
> +int ret;
> +
> +VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s",
> + mon, device, file, format);
> +
> +if (!mon) {
> +qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +_("monitor must not be NULL"));
> +return -1;
> +}
> +
> +if (mon->json)
> +ret = qemuMonitorJSONDriveReopen(mon, device, file, format);
> +else
> +qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +_("drive-reopen requires JSON monitor"));
You should set ret to -1 here (or return -1).
> +return ret;
> +}
> +
> int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
> const char *cmd,
> char **reply,
--
Federico
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [vdsm] oVirt Live Snapshots
- Original Message - > From: "Shu Ming" > To: "Federico Simoncelli" > Cc: qemu-de...@nongnu.org, libvir-list@redhat.com, "VDSM Project Development" > , > "Dave Allan" , "Eric Blake" > Sent: Thursday, February 2, 2012 1:59:01 PM > Subject: Re: [vdsm] oVirt Live Snapshots > > Can someone explain what is "DB" in this wiki page? It is the ovirt-engine database, where the VMs/images information and status is stored. That part of the wiki should be improved. > See, > > > Live snapshots operation extend regular snapshots as follow: > > • Create a locked snapshot in DB -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] oVirt Live Snapshots
Hi, oVirt, and more specifically VDSM, is currently implementing the live snapshot feature using the API/commands provided by libvirt and qemu. It would be great if you could review the design and the current open issues at: http://ovirt.org/wiki/Live_Snapshots Thank you, -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol
- Original Message -
> From: "Jiri Denemark"
> To: libvir-list@redhat.com
> Sent: Monday, January 23, 2012 2:30:54 PM
> Subject: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol
>
> We already provide ways to detect when a domain has been paused as a
> result of I/O error, but there was no way of getting the exact error
> or
> even the device that experienced it. This new API may be used for
> both.
> ---
> include/libvirt/libvirt.h.in | 19 +++
> src/driver.h |6
> src/libvirt.c| 53
> ++
> src/libvirt_public.syms |5
> src/remote/remote_driver.c |1 +
> src/remote/remote_protocol.x | 13 +-
> src/remote_protocol-structs |9 +++
> 7 files changed, 105 insertions(+), 1 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in
> b/include/libvirt/libvirt.h.in
> index 958e5a6..2c3423a 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn,
> int interval,
> unsigned int count);
>
> +/**
> + * virDomainIoError:
> + *
> + * Disk I/O error.
> + */
> +typedef enum {
> +VIR_DOMAIN_IOERROR_NONE = 0, /* no error */
> +VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device
> */
> +VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */
> +
> +#ifdef VIR_ENUM_SENTINELS
> +VIR_DOMAIN_IOERROR_LAST
> +#endif
> +} virDomainIoError;
> +
>
Hi Jiri, how do we detect an EIO error? We should need an additional
error type here?
--
Federico
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort
- Original Message - > From: "Adam Litke" > To: "Dave Allan" > 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? > 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? -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] sanlock: avoid lockspace setup when auto_disk_lease is off
When auto_disk_lease is off we should avoid the automatic lockspace
creation.
Signed-off-by: Federico Simoncelli
---
src/locking/lock_driver_sanlock.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index cd2bbb5..29d4176 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -294,8 +294,10 @@ static int virLockManagerSanlockInit(unsigned int version,
goto error;
}
-if (virLockManagerSanlockSetupLockspace() < 0)
-goto error;
+if (driver->autoDiskLease) {
+if (virLockManagerSanlockSetupLockspace() < 0)
+goto error;
+}
return 0;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] Introduce virDomainGetControlInfo API
Hi Jiri,
I don't see the libvirt_virDomainGetControlInfo implementation in
libvirt-override.c.
$ nm -D python/.libs/libvirtmod.so | grep virDomainGetControlInfo
(empty)
Is the python binding postponed to a future patch?
--
Federico
On Tue, Jun 7, 2011 at 3:01 PM, Jiri Denemark wrote:
> The API can be used to query current state of an interface to VMM used
> to control a domain. In QEMU world this translates into monitor
> connection.
> ---
> include/libvirt/libvirt.h.in | 40
> python/generator.py | 1 +
> python/libvirt-override-api.xml | 6 +
> src/driver.h | 5
> src/libvirt.c | 48
> +++
> src/libvirt_public.syms | 5
> 6 files changed, 105 insertions(+), 0 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index df213f1..1454ca0 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -141,6 +141,43 @@ typedef enum {
> VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */
> } virDomainCrashedReason;
>
> +
> +/**
> + * virDomainControlState:
> + *
> + * Current state of a control interface to the domain.
> + */
> +typedef enum {
> + VIR_DOMAIN_CONTROL_OK = 0, /* operational, ready to accept
> commands */
> + VIR_DOMAIN_CONTROL_JOB = 1, /* background job is running (can be
> + monitored by virDomainGetJobInfo);
> only
> + limited set of commands may be
> allowed */
> + VIR_DOMAIN_CONTROL_OCCUPIED = 2, /* occupied by a running command */
> + VIR_DOMAIN_CONTROL_ERROR = 3, /* unusable, domain cannot be fully
> operated */
> +} virDomainControlState;
> +
> +/**
> + * virDomainControlInfo:
> + *
> + * Structure filled in by virDomainGetControlInfo and providing details about
> + * current state of control interface to a domain.
> + */
> +typedef struct _virDomainControlInfo virDomainControlInfo;
> +struct _virDomainControlInfo {
> + unsigned int state; /* control state, one of virDomainControlState */
> + unsigned int details; /* state details, currently 0 */
> + unsigned long long stateTime; /* for how long (in msec) control interface
> + has been in current state (except for OK
> + and ERROR states) */
> +};
> +
> +/**
> + * virDomainControlInfoPtr:
> + *
> + * Pointer to virDomainControlInfo structure.
> + */
> +typedef virDomainControlInfo *virDomainControlInfoPtr;
> +
> /**
> * virDomainModificationImpact:
> *
> @@ -781,6 +818,9 @@ int virDomainGetState
> (virDomainPtr domain,
> int *state,
> int *reason,
> unsigned int flags);
> +int virDomainGetControlInfo (virDomainPtr domain,
> + virDomainControlInfoPtr
> info,
> + unsigned int flags);
>
> /*
> * Return scheduler type in effect 'sedf', 'credit', 'linux'
> diff --git a/python/generator.py b/python/generator.py
> index 7c38fdd..4e3e9fa 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -306,6 +306,7 @@ skip_impl = (
> 'virGetLastError',
> 'virDomainGetInfo',
> 'virDomainGetState',
> + 'virDomainGetControlInfo',
> 'virDomainGetBlockInfo',
> 'virDomainGetJobInfo',
> 'virNodeGetInfo',
> diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
> index ec08e69..01207d6 100644
> --- a/python/libvirt-override-api.xml
> +++ b/python/libvirt-override-api.xml
> @@ -54,6 +54,12 @@
>
>
>
> +
> + Extract details about current state of control interface to a
> domain.
> +
> +
> +
> +
>
> Extract information about a domain block device size
>
> diff --git a/src/driver.h b/src/driver.h
> index 5df798a..ca49b08 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -171,6 +171,10 @@ typedef int
> int *reason,
> unsigned int flags);
> typedef int
> + (*virDrvDomainGetControlInfo) (virDomainPtr domain,
> + virDomainControlInfoPtr info,
> + unsigned int flags);
> +typedef int
> (*virDrvDomainSave) (virDomainPtr domain,
> const char *to);
> typedef int
> @@ -663,6 +667,7 @@ struct _virDriver {
> virDrvDomainGetBlkioParameters domainGetBlkioParameters;
> virDrvDomainGetInfo domainGetInfo;
> virDrvDomainGetState domainGetState;
> + virDrvDomainG
[libvirt] [PATCHv5] qemu: allow blkstat/blkinfo calls during migration
If this is correct I'll squash it with the v4 patch.
---
src/qemu/qemu_domain.h|4 ++--
src/qemu/qemu_driver.c|6 ++
src/qemu/qemu_migration.c |8
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index af513e7..effaebc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -56,10 +56,10 @@ struct qemuDomainJobSignalsData {
unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED
*/
char *statDevName; /* Device name used by blkstat calls */
virDomainBlockStatsPtr blockStat; /* Block statistics for
QEMU_JOB_SIGNAL_BLKSTAT */
-int statRetCode; /* Return code for the blkstat calls */
+int *statRetCode; /* Return code for the blkstat calls */
char *infoDevName; /* Device name used by blkinfo calls */
virDomainBlockInfoPtr blockInfo; /* Block information for
QEMU_JOB_SIGNAL_BLKINFO */
-int infoRetCode; /* Return code for the blkinfo calls */
+int *infoRetCode; /* Return code for the blkinfo calls */
};
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a4e430b..d4287dc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5298,13 +5298,12 @@ qemudDomainBlockStats (virDomainPtr dom,
priv->jobSignalsData.statDevName = disk->info.alias;
priv->jobSignalsData.blockStat = stats;
-priv->jobSignalsData.statRetCode = -1;
+priv->jobSignalsData.statRetCode = &ret;
priv->jobSignals |= QEMU_JOB_SIGNAL_BLKSTAT;
while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT)
ignore_value(virCondWait(&priv->signalCond, &vm->lock));
-ret = priv->jobSignalsData.statRetCode;
if (virDomainObjUnref(vm) == 0)
vm = NULL;
} else {
@@ -5745,13 +5744,12 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
priv->jobSignalsData.infoDevName = disk->info.alias;
priv->jobSignalsData.blockInfo = info;
-priv->jobSignalsData.infoRetCode = -1;
+priv->jobSignalsData.infoRetCode = &ret;
priv->jobSignals |= QEMU_JOB_SIGNAL_BLKINFO;
while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKINFO)
ignore_value(virCondWait(&priv->signalCond, &vm->lock));
-ret = priv->jobSignalsData.infoRetCode;
if (virDomainObjUnref(vm) == 0)
vm = NULL;
} else {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 28b168c..b767fb7 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -659,8 +659,8 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver,
job, _("guest unexpectedly quit"));
if (cleanup) {
priv->jobSignals = 0;
-priv->jobSignalsData.statRetCode = -1;
-priv->jobSignalsData.infoRetCode = -1;
+priv->jobSignalsData.statRetCode = NULL;
+priv->jobSignalsData.infoRetCode = NULL;
}
return -1;
}
@@ -712,7 +712,7 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver,
&priv->jobSignalsData.blockStat->errs);
qemuDomainObjExitMonitorWithDriver(driver, vm);
-priv->jobSignalsData.statRetCode = ret;
+*priv->jobSignalsData.statRetCode = ret;
priv->jobSignals ^= QEMU_JOB_SIGNAL_BLKSTAT;
if (ret < 0)
@@ -724,7 +724,7 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver,
&priv->jobSignalsData.blockInfo->allocation);
qemuDomainObjExitMonitorWithDriver(driver, vm);
-priv->jobSignalsData.infoRetCode = ret;
+*priv->jobSignalsData.infoRetCode = ret;
priv->jobSignals ^= QEMU_JOB_SIGNAL_BLKINFO;
if (ret < 0)
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
Originally most of libvirt domain-specific calls were blocking
during a migration.
A new mechanism to allow specific calls (blkstat/blkinfo) to be
executed in such condition has been implemented.
In the long term it'd be desirable to get a more general
solution to mark further APIs as migration safe, without needing
special case code.
* src/qemu/qemu_migration.c: add some additional job signal
flags for doing blkstat/blkinfo during a migration
* src/qemu/qemu_domain.c: add a condition variable that can be
used to efficiently wait for the migration code to clear the
signal flag
* src/qemu/qemu_driver.c: execute blkstat/blkinfo using the
job signal flags during migration
---
src/qemu/qemu_domain.c| 13 ++
src/qemu/qemu_domain.h|9
src/qemu/qemu_driver.c| 103 -
src/qemu/qemu_migration.c | 38 -
4 files changed, 131 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0ac4861..ee029c5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -110,7 +110,19 @@ static void *qemuDomainObjPrivateAlloc(void)
if (VIR_ALLOC(priv) < 0)
return NULL;
+if (virCondInit(&priv->jobCond) < 0)
+goto initfail;
+
+if (virCondInit(&priv->signalCond) < 0) {
+ignore_value(virCondDestroy(&priv->jobCond));
+goto initfail;
+}
+
return priv;
+
+initfail:
+VIR_FREE(priv);
+return NULL;
}
static void qemuDomainObjPrivateFree(void *data)
@@ -123,6 +135,7 @@ static void qemuDomainObjPrivateFree(void *data)
virDomainChrSourceDefFree(priv->monConfig);
VIR_FREE(priv->vcpupids);
VIR_FREE(priv->lockState);
+ignore_value(virCondDestroy(&priv->signalCond));
/* This should never be non-NULL if we get here, but just in case... */
if (priv->mon) {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0fca974..a07a500 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -47,11 +47,19 @@ enum qemuDomainJobSignals {
QEMU_JOB_SIGNAL_SUSPEND = 1 << 1, /* Request VM suspend to finish live
migration offline */
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 << 2, /* Request migration downtime
change */
QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 << 3, /* Request migration speed change
*/
+QEMU_JOB_SIGNAL_BLKSTAT = 1 << 4, /* Request blkstat during migration */
+QEMU_JOB_SIGNAL_BLKINFO = 1 << 5, /* Request blkinfo during migration */
};
struct qemuDomainJobSignalsData {
unsigned long long migrateDowntime; /* Data for
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME */
unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED
*/
+char *statDevName; /* Device name used by blkstat calls */
+virDomainBlockStatsPtr blockStat; /* Block statistics for
QEMU_JOB_SIGNAL_BLKSTAT */
+int statRetCode; /* Return code for the blkstat calls */
+char *infoDevName; /* Device name used by blkinfo calls */
+virDomainBlockInfoPtr blockInfo; /* Block information for
QEMU_JOB_SIGNAL_BLKINFO */
+int infoRetCode; /* Return code for the blkinfo calls */
};
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
@@ -61,6 +69,7 @@ typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */
+virCond signalCond; /* Use to coordinate the safe queries during migration
*/
enum qemuDomainJob jobActive; /* Currently running job */
unsigned int jobSignals;/* Signals for running job */
struct qemuDomainJobSignalsData jobSignalsData; /* Signal specific data */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 30ee79e..5987ed4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5104,15 +5104,6 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
-if (qemuDomainObjBeginJob(vm) < 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
-goto endjob;
-}
-
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(path, vm->def->disks[i]->dst)) {
disk = vm->def->disks[i];
@@ -5123,29 +5114,57 @@ qemudDomainBlockStats (virDomainPtr dom,
if (!disk) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("invalid path: %s"), path);
-goto endjob;
+goto cleanup;
}
if (!disk->info.alias) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
-goto endjob;
+goto cleanup;
}
priv = vm->privateData;
-qemuDomainObjEnterMonitor(vm);
-ret = qemuMonitorGetBlockStatsInfo(priv->mon,
-
Re: [libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
- Original Message -
> From: "Eric Blake"
> To: "Federico Simoncelli"
> Cc: libvir-list@redhat.com
> Sent: Friday, May 13, 2011 1:38:45 AM
> Subject: Re: [PATCH] qemu: allow blkstat/blkinfo calls during migration
> On 05/11/2011 07:26 AM, Federico Simoncelli wrote:
> >
> > + if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT)
> > + || (priv->jobActive == QEMU_JOB_SAVE)) {
> > + virDomainObjRef(vm);
> > + while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT)
> > + ignore_value(virCondWait(&priv->signalCond, &vm->lock));
>
> Hmm, should we mark priv->jobSignals as volatile in the header, to
> ensure the compiler won't optimize this into an infinite loop?
Not sure.
http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/
> > + if (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) {
> > + qemuDomainObjEnterMonitorWithDriver(driver, vm);
> > + rc = qemuMonitorGetBlockStatsInfo(priv->mon,
>
> I'm still wondering if we need to hold the signalLock condition during
> the duration where we drop driver lock to call out to the monitor. I
> can't convince myself that we need to, but I also can't convince
> myself
> that your code is safe without it (I guess it goes to show that I
> haven't done much programming on condition variables in any prior job
> -
> they're cool tools, but hard to wrap your head around when first
> learning them).
There is no signalLock, on the other hand the signalCond is used to wake
up a thread (using virCondSignal/virCondBroadcast) that is currently
sleeping on virCondWait (and not holding vm->lock).
--
Federico
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
Originally most of libvirt domain-specific calls were blocking
during a migration.
A new mechanism to allow specific calls (blkstat/blkinfo) to be
executed in such condition has been implemented.
In the long term it'd be desirable to get a more general
solution to mark further APIs as migration safe, without needing
special case code.
* src/qemu/qemu_migration.c: add some additional job signal
flags for doing blkstat/blkinfo during a migration
* src/qemu/qemu_domain.c: add a condition variable that can be
used to efficiently wait for the migration code to clear the
signal flag
* src/qemu/qemu_driver.c: execute blkstat/blkinfo using the
job signal flags during migration
---
src/qemu/qemu_domain.c|6 +++
src/qemu/qemu_domain.h|9
src/qemu/qemu_driver.c| 103 -
src/qemu/qemu_migration.c | 35 +++
4 files changed, 123 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c61f9bf..570d65e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -110,6 +110,11 @@ static void *qemuDomainObjPrivateAlloc(void)
if (VIR_ALLOC(priv) < 0)
return NULL;
+if (virCondInit(&priv->signalCond) < 0) {
+VIR_FREE(priv);
+return NULL;
+}
+
return priv;
}
@@ -122,6 +127,7 @@ static void qemuDomainObjPrivateFree(void *data)
qemuDomainPCIAddressSetFree(priv->pciaddrs);
virDomainChrSourceDefFree(priv->monConfig);
VIR_FREE(priv->vcpupids);
+ignore_value(virCondDestroy(&priv->signalCond));
/* This should never be non-NULL if we get here, but just in case... */
if (priv->mon) {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6d24f53..af513e7 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -47,11 +47,19 @@ enum qemuDomainJobSignals {
QEMU_JOB_SIGNAL_SUSPEND = 1 << 1, /* Request VM suspend to finish live
migration offline */
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 << 2, /* Request migration downtime
change */
QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 << 3, /* Request migration speed change
*/
+QEMU_JOB_SIGNAL_BLKSTAT = 1 << 4, /* Request blkstat during migration */
+QEMU_JOB_SIGNAL_BLKINFO = 1 << 5, /* Request blkinfo during migration */
};
struct qemuDomainJobSignalsData {
unsigned long long migrateDowntime; /* Data for
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME */
unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED
*/
+char *statDevName; /* Device name used by blkstat calls */
+virDomainBlockStatsPtr blockStat; /* Block statistics for
QEMU_JOB_SIGNAL_BLKSTAT */
+int statRetCode; /* Return code for the blkstat calls */
+char *infoDevName; /* Device name used by blkinfo calls */
+virDomainBlockInfoPtr blockInfo; /* Block information for
QEMU_JOB_SIGNAL_BLKINFO */
+int infoRetCode; /* Return code for the blkinfo calls */
};
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
@@ -61,6 +69,7 @@ typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */
+virCond signalCond; /* Use to coordinate the safe queries during migration
*/
enum qemuDomainJob jobActive; /* Currently running job */
unsigned int jobSignals;/* Signals for running job */
struct qemuDomainJobSignalsData jobSignalsData; /* Signal specific data */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f288d3..f7b2cb4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5082,15 +5082,6 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
-if (qemuDomainObjBeginJob(vm) < 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
-goto endjob;
-}
-
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(path, vm->def->disks[i]->dst)) {
disk = vm->def->disks[i];
@@ -5101,29 +5092,57 @@ qemudDomainBlockStats (virDomainPtr dom,
if (!disk) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("invalid path: %s"), path);
-goto endjob;
+goto cleanup;
}
if (!disk->info.alias) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
-goto endjob;
+goto cleanup;
}
priv = vm->privateData;
-qemuDomainObjEnterMonitor(vm);
-ret = qemuMonitorGetBlockStatsInfo(priv->mon,
- disk->info.alias,
- &stats->rd_req,
- &stats->rd_bytes,
-
Re: [libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
Hi Eric, thank you for the quick and detailed patch review! I think I fixed all the problems that you described but if you find anything that I missed or anything new I'll be happy to accept the help you offered :) You can update my email address in AUTHORS to the redhat one. commit a0ca40ce07f6c54901aac4e32bcfd573980bd38f Author: Federico Simoncelli Date: Tue May 10 11:36:48 2011 +0100 qemu: allow blkstat/blkinfo calls during migration Originally most of libvirt domain-specific calls were blocking during a migration. A new mechanism to allow specific calls (blkstat/blkinfo) to be executed in such condition has been implemented. In the long term it'd be desirable to get a more general solution to mark further APIs as migration safe, without needing special case code. * src/qemu/qemu_migration.c: add some additional job signal flags for doing blkstat/blkinfo during a migration * src/qemu/qemu_domain.c: add a condition variable that can be used to efficiently wait for the migration code to clear the signal flag * src/qemu/qemu_driver.c: execute blkstat/blkinfo using the job signal flags during migration --- src/qemu/qemu_domain.c|6 +++ src/qemu/qemu_domain.h|9 src/qemu/qemu_driver.c| 103 - src/qemu/qemu_migration.c | 35 +++ 4 files changed, 123 insertions(+), 30 deletions(-) -- Federico. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
References:
- https://www.redhat.com/archives/libvir-list/2011-May/msg00210.html
- https://www.redhat.com/archives/libvir-list/2011-May/msg00287.html
---
src/qemu/qemu_domain.c|6 +++
src/qemu/qemu_domain.h|7
src/qemu/qemu_driver.c| 86 +++--
src/qemu/qemu_migration.c | 31
4 files changed, 103 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c61f9bf..d4e53c4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -526,6 +526,12 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
priv->jobStart = timeval_to_ms(now);
memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
+if (virCondInit(&priv->signalCond) < 0) {
+virReportSystemError(errno,
+ "%s", _("cannot initialize signal condition"));
+return -1;
+}
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6d24f53..b82986c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -47,11 +47,17 @@ enum qemuDomainJobSignals {
QEMU_JOB_SIGNAL_SUSPEND = 1 << 1, /* Request VM suspend to finish live
migration offline */
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 << 2, /* Request migration downtime
change */
QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 << 3, /* Request migration speed change
*/
+QEMU_JOB_SIGNAL_BLKSTAT = 1 << 4, /* Request blkstat during migration */
+QEMU_JOB_SIGNAL_BLKINFO = 1 << 5, /* Request blkinfo during migration */
};
struct qemuDomainJobSignalsData {
unsigned long long migrateDowntime; /* Data for
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME */
unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED
*/
+char *devname; /* Device name used by blkstat/blkinfo calls */
+int returnCode; /* Return code for the blkstat/blkinfo calls */
+virDomainBlockStatsPtr blockStat; /* Block statistics for
QEMU_JOB_SIGNAL_BLKSTAT */
+virDomainBlockInfoPtr blockInfo; /* Block information for
QEMU_JOB_SIGNAL_BLKINFO */
};
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
@@ -61,6 +67,7 @@ typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */
+virCond signalCond; /* Use in conjunction with main virDomainObjPtr lock */
enum qemuDomainJob jobActive; /* Currently running job */
unsigned int jobSignals;/* Signals for running job */
struct qemuDomainJobSignalsData jobSignalsData; /* Signal specific data */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0fd0f10..f9f5e83 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5031,13 +5031,10 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
-if (qemuDomainObjBeginJob(vm) < 0)
-goto cleanup;
-
if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
-goto endjob;
+goto cleanup;
}
for (i = 0 ; i < vm->def->ndisks ; i++) {
@@ -5050,29 +5047,48 @@ qemudDomainBlockStats (virDomainPtr dom,
if (!disk) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("invalid path: %s"), path);
-goto endjob;
+goto cleanup;
}
if (!disk->info.alias) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
-goto endjob;
+goto cleanup;
}
priv = vm->privateData;
-qemuDomainObjEnterMonitor(vm);
-ret = qemuMonitorGetBlockStatsInfo(priv->mon,
- disk->info.alias,
- &stats->rd_req,
- &stats->rd_bytes,
- &stats->wr_req,
- &stats->wr_bytes,
- &stats->errs);
-qemuDomainObjExitMonitor(vm);
+if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT)
+|| (priv->jobActive == QEMU_JOB_SAVE)) {
-endjob:
-if (qemuDomainObjEndJob(vm) == 0)
-vm = NULL;
+while (priv->jobSignals)
+ignore_value(virCondWait(&priv->signalCond, &vm->lock));
+
+priv->jobSignals |= QEMU_JOB_SIGNAL_BLKSTAT;
+priv->jobSignalsData.devname = disk->info.alias;
+priv->jobSignalsData.blockStat = stats;
+priv->jobSignalsData.returnCode = -1;
+
+while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT)
+ignore_value(virCondWait(&priv->signalCond, &vm->lock));
+
+ret = priv->jobSignalsData.returnCode;
+} else {
+if (qemuDomainObjBeginJob(vm) < 0)
+goto cleanup;
+
+qemuDomainObjE
[libvirt] [PATCH] Fixing issues in re-detection of transient VMs
Re-detection of transient VMs had two issues described in bugs 507304 and 507537: https://bugzilla.redhat.com/show_bug.cgi?id=507304 Summary: After a migration the domain status is saved as "paused" and it is not updated to "running". A following libvirtd restart will detect the vm as "paused" (instead of "running"). https://bugzilla.redhat.com/show_bug.cgi?id=507537 Summary: Destroying a transient re-detected domain leaves the domain defined (instead of completely remove it). Patches in attachment (and available in the bug report). -- Federico Simoncelli. libvirt-0.6.5-save-status-on-migration.patch Description: Binary data libvirt-0.6.5-fix-destroy-after-restart.patch Description: Binary data -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Latest kvm packages for CentOS 5.3
Hi all, I've been working quite extensively with kvm on CentOS 5.3 lately. If you are interested in the latest rpm of kvm-kmod-2.6.30-rc8, qemu-kvm-0.10.5 and libvirt-0.6.4 you can temporary find them here: http://update.nethesis.it/kvm/ I've had no problem so far using these packages. Feedback is welcome. -- Federico. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
