[libvirt] [PATCH] Quieten virsh schedinfo for shutoff domain
# HG changeset patch # User john.levon@sun.com # Date 1232675291 28800 # Node ID 3ef027308b88b47b2f3ca721bf88f2e606d8e3bc # Parent 903a29e99c80a35ce7f4754a3bcc3ab34bf32d8a Quieten virsh schedinfo for shutoff domain Quietly return failure for a shutoff domain in the sched params driver routines. Signed-off-by: John Levon <john.levon@sun.com> diff --git a/src/xen_internal.c b/src/xen_internal.c --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1069,11 +1069,17 @@ xenHypervisorGetSchedulerType(virDomainP } priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) { + if (priv->handle < 0) { virXenErrorFunc(domain->conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__, - "priv->handle or domain->id invalid", 0); + "priv->handle invalid", 0); return NULL; } + + /* + * If it's not running, we can't help. + */ + if (domain->id < 0) + return NULL; /* * Support only dom_interface_version >=5 @@ -1144,11 +1150,17 @@ xenHypervisorGetSchedulerParameters(virD } priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) { + if (priv->handle < 0) { virXenErrorFunc(domain->conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__, - "priv->handle or domain->id invalid", 0); + "priv->handle invalid", 0); return -1; } + + /* + * If it's not running, we can't help. + */ + if (domain->id < 0) + return -1; /* * Support only dom_interface_version >=5 @@ -1242,11 +1254,17 @@ xenHypervisorSetSchedulerParameters(virD } priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || domain->id < 0) { + if (priv->handle < 0) { virXenErrorFunc (domain->conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__, - "priv->handle or domain->id invalid", 0); + "priv->handle invalid", 0); return -1; } + + /* + * If it's not running, we can't help. + */ + if (domain->id < 0) + return -1; /* * Support only dom_interface_version >=5
On Thu, Jan 22, 2009 at 05:49:12PM -0800, john.levon@sun.com wrote:
+ /* + * If it's not running, we can't help. + */ + if (domain->id < 0) + return NULL;
NACK. Unfortunately you shouldn't return from a function without setting an error (and you have to set an error exactly once otherwise earlier errors get overwritten). Perhaps if you want to silence the error, you can silence it in the calling code, eg. in virsh, by matching on the appropriate virterror fields, eg error->code ? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Mon, Jan 26, 2009 at 10:54:23AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 22, 2009 at 05:49:12PM -0800, john.levon@sun.com wrote:
+ /* + * If it's not running, we can't help. + */ + if (domain->id < 0) + return NULL;
NACK. Unfortunately you shouldn't return from a function without setting an error (and you have to set an error exactly once otherwise earlier errors get overwritten).
Perhaps if you want to silence the error, you can silence it in the calling code, eg. in virsh, by matching on the appropriate virterror fields, eg error->code ?
The calling code isn't in virsh, it's in xen_unified. The XenStore driver indeed cannot deal with non-running domains. I maintain this is the correct behaviour for a sub-driver (a driver should certainly set an error). We already have enough problems with overly eager error reporting (try a virsh start non-existent-dom !), let's not make it worse. regards john
participants (3)
-
John Levon -
john.levon＠sun.com
-
Richard W.M. Jones