6:35 a.m.
Integrating feedback to the original version.
This changes the approach to fixing the problem - previously, we called
virDomainGetBlockInfo() and if it failed, we mitigated the failure for CDROMs
and floppies specifically. Now we first check for existence of a <source>
element in the corresponding XML and if we find none, we avoid calling
virDomainGetBlockInfo() altogether as we know it's bound to fail in that case.
The benefit is that whereas the previous fix swallowed all errors concerning
CDROMs and floppies - not just missing <source> - this one only handles
missing <source> specifically and doesn't mask other problems that might come
up.
The patch itself is fairly simple, unfortunately some noise is caused by
additional indentation of code related to the virDomainGetBlockInfo() call.
That code should however be unchanged (apart from reformating a comment to
keep line lengths under 80 chars).
Pavel Mores (1):
fixed handling of sourceless disks in 'domblkinfo' cmd
tools/virsh-domain-monitor.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
--
2.21.0
6:35 a.m.
New subject: [libvirt] [PATCH v2 1/1] fixed handling of sourceless disks in 'domblkinfo' cmd
virDomainGetBlockInfo() returns error if called on a disk with no
source (a sourceless disk might be a removable media drive with no
media in it, for instance an empty CDROM or floppy drive).
So far this caused the virsh domblkinfo --all command to abort and
ignore any remaining (not yet displayed) disk devices. This patch
fixes the problem by first checking for existence of a <source>
element in the corresponding XML. If none is found, we avoid calling
virDomainGetBlockInfo() altogether as we know it's bound to fail in
that case.
https://bugzilla.redhat.com/show_bug.cgi?id=1619625
Signed-off-by: Pavel Mores <pmores(a)redhat.com>
---
tools/virsh-domain-monitor.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 0e2c4191d7..b69283b2da 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -507,20 +507,27 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
protocol = virXPathString("string(./source/@protocol)", ctxt);
target = virXPathString("string(./target/@dev)", ctxt);
- rc = virDomainGetBlockInfo(dom, target, &info, 0);
-
- if (rc < 0) {
- /* If protocol is present that's an indication of a networked
- * storage device which cannot provide statistics, so generate
- * 0 based data and get the next disk. */
- if (protocol && !active &&
- virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
- virGetLastErrorDomain() == VIR_FROM_STORAGE) {
- memset(&info, 0, sizeof(info));
- vshResetLibvirtError();
- } else {
- goto cleanup;
+ if (virXPathBoolean("boolean(./source)", ctxt) == 1) {
+
+ rc = virDomainGetBlockInfo(dom, target, &info, 0);
+
+ if (rc < 0) {
+ /* If protocol is present that's an indication of a
+ * networked storage device which cannot provide statistics,
+ * so generate 0 based data and get the next disk. */
+ if (protocol && !active &&
+ virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
+ virGetLastErrorDomain() == VIR_FROM_STORAGE) {
+ memset(&info, 0, sizeof(info));
+ vshResetLibvirtError();
+ } else {
+ goto cleanup;
+ }
}
+ } else {
+ /* if we don't call virDomainGetBlockInfo() who clears
'info'
+ * we have to do it manually */
+ memset(&info, 0, sizeof(info));
}
if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy,
human))
--
2.21.0
7:13 a.m.
New subject: [libvirt] [PATCH v2 1/1] fixed handling of sourceless disks in 'domblkinfo' cmd
On Fri, Oct 04, 2019 at 01:35:09PM +0200, Pavel Mores wrote:
virDomainGetBlockInfo() returns error if called on a disk with no
source (a sourceless disk might be a removable media drive with no
media in it, for instance an empty CDROM or floppy drive).
So far this caused the virsh domblkinfo --all command to abort and
ignore any remaining (not yet displayed) disk devices. This patch
fixes the problem by first checking for existence of a <source>
element in the corresponding XML. If none is found, we avoid calling
virDomainGetBlockInfo() altogether as we know it's bound to fail in
that case.
https://bugzilla.redhat.com/show_bug.cgi?id=1619625
Signed-off-by: Pavel Mores <pmores(a)redhat.com>
---
tools/virsh-domain-monitor.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1873
days inactive
1876
days old
2 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Mores