[libvirt] [PATCH] virsh: Don't fetch status for all domains in cmdList

We are getting the list of domains and after that we iterate over the list and try to get status for each domain hoping it will skip over domains that disappeared meanwhile. However, this solution to race is bogus - domain may disappear right after we have checked its state and before we exec another API over it (e.g. virDomainHasManagedSaveImage()). Also, when printing just names or uuids (list --name / --uuid) we issue APIs to obtain the values, however these require no RPC call as all requested info is in virDomain object that client already has. Therefore move the status obtaining only to the place that really needs it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 7eb387d..7dcbc5c 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1876,17 +1876,17 @@ cmdList(vshControl *ctl, const vshCmd *cmd) else ignore_value(virStrcpyStatic(id_buf, "-")); - state = virshDomainState(ctl, dom, NULL); - - /* Domain could've been removed in the meantime */ - if (state < 0) - continue; - - if (optTable && managed && state == VIR_DOMAIN_SHUTOFF && - virDomainHasManagedSaveImage(dom, 0) > 0) - state = -2; - if (optTable) { + state = virshDomainState(ctl, dom, NULL); + + /* Domain could've been removed in the meantime */ + if (state < 0) + continue; + + if (managed && state == VIR_DOMAIN_SHUTOFF && + virDomainHasManagedSaveImage(dom, 0) > 0) + state = -2; + if (optTitle) { if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) goto cleanup; -- 2.4.10

On Wed, Jan 20, 2016 at 04:11:26PM +0100, Michal Privoznik wrote:
We are getting the list of domains and after that we iterate over the list and try to get status for each domain hoping it will skip over domains that disappeared meanwhile. However, this solution to race is bogus - domain may disappear right after we have checked its state and before we exec another API over it (e.g. virDomainHasManagedSaveImage()). Also, when printing just
There seems to be a similar problem in virshDomainListCollect, but it is only used with old libvirtd that does not support the atomic listing API.
names or uuids (list --name / --uuid) we issue APIs to obtain the values, however these require no RPC call as all requested info is in virDomain object that client already has. Therefore move the status obtaining only to the place that really needs it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
ACK Jan
participants (2)
-
Ján Tomko
-
Michal Privoznik