B #4164: Use xmllint to test disk target device #4168
Conversation
rsmontero
added
Category: API
Category: KVM
Community
Priority: Normal
Type: Feature
and removed
Category: API
labels
src/vmm_mad/remotes/kvm/detach_disk
Outdated
| @@ -29,9 +29,10 @@ DETACH_PARAMS="--domain $DOMAIN --target $TARGET" | |||
| exec_and_log "virsh --connect $LIBVIRT_URI detach-disk $DETACH_PARAMS" \ | |||
| "Could not detach $TARGET from $DOMAIN" | |||
|
|
|||
| virsh --connect $LIBVIRT_URI dumpxml $DOMAIN | grep $TARGET > /dev/null 2>&1 | |||
| virsh --connect $LIBVIRT_URI dumpxml $DOMAIN | \ | |||
| xmllint -xpath "//disk/target[@dev='$TARGET']" >/dev/null 2>&1 | |||
There was a problem hiding this comment.
This will adds xmllint to dependencies, wouldn't be better to use /var/tmp/one/datastore/xpath.rb instead?
There was a problem hiding this comment.
The following reasons:
xmllintis part of thelibxml2on which severallibvirtsub-packages depends- the exit code of
xpath.rbcould not be used. I hate adding unnecessary piping/processing just to get an exit status.
There was a problem hiding this comment.
... and augeas ;-)
So it is already available due to required dependencies.
There was a problem hiding this comment.
True, so I also prefer using xmllint. One small thing, there seems to be missing - (hyphen sign) as a last xmllint parameter to even process the stdin.
There was a problem hiding this comment.
hm. stupid typo :)
how do you prefer - to push second commit or edit and force-push?
There was a problem hiding this comment.
Doesn't really matter, you can force-push or add another commit, which will be squashed when merging.
Signed-off-by: Anton Todorov <a.todorov@storpool.com>
atodorov-storpool
force-pushed
the
bug-4164
branch
from
741c35d to
2ff6fe8
Compare
Signed-off-by: Anton Todorov a.todorov@storpool.com