On 08/27/2013 05:54 PM, Peter Krempa wrote:
On 08/27/13 11:37, Michal Privoznik wrote:
On 27.08.2013 10:58, Alex Jia wrote:
On 08/27/2013 04:47 PM, Peter Krempa wrote:
On 08/27/13 09:53, Alex Jia wrote:
The flag "VIR_DOMAIN_BLOCK_COMMIT_DELETE" is missed by
qemuDomainBlockCommit(),
and then will hit error "unsupported flags (0x2) in function
qemuDomainBlockCommit" if users run 'virsh blockcommit' with
'--delete' option.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1001475
Signed-off-by: Alex Jia<ajia@xxxxxxxxxx>
---
src/qemu/qemu_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ed29373..8863124 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14444,7 +14444,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const
char *path, const char *base,
const char *base_canon = NULL;
bool clean_access = false;
- virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
+ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_DELETE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
The code doesn't seem to support the BLOCK_COMMIT_DELETE flag. It was
Yes, the codes haven't any implementation for BLOCK_COMMIT_DELETE flag
now, maybe, only need to raise a friendly error message in here instead
of "unsupported flags (0x2) xxxx".
I agree that this error message is not user-friendly. Bare virsh users
know nothing about our flags and their numerical expression. However, I
don't think there is a way how to produce "Unsupported flag
VIR_DOMAIN_BLOCK_COMMIT_DELETE" instead of "Unsupported flag 0x2" since
all we see in the qemuDomainBlockCommit() function is just number. I
mean, mapping of flag onto numeric value is not one-to-one function (aka
injective function). That is, a value 0x2 can express
VIR_DOMAIN_BLOCK_COMMIT_DELETE, VIR_DOMAIN_START_AUTODESTROY,
VIR_DUMP_DESTROY, etc. (git grep "1<< 1," include/).
If we want to make it work, we have to introduce an injective function,
e.g. virUnsupportedFlags(), which would accept a list (not an ORed
value) of all flags that are not supported. Too much effort for not much
outcome.
Additionally this would require updating the list of unsupported flags
at each function when adding a new flag to keep them in sync or add a
way to autogenerate the list automagically.
It would be nice to have this automatic, but I think that Alex was
suggesting something like
if (flags& VIR_DOMAIN_BLOCK_COMMIT_DELETE) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "we don't support this yet");
goto cleanup;
}
Exactly, but I think it's only a temporary solution, the Michal's
suggestion is more
reasonable, and your solution looks like more intelligent :)
Peter
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list