Hi,
Am 28.08.2013 13:48, schrieb Kevin Wolf:
Am 28.08.2013 um 13:39 hat Max Reitz geschrieben:
+ /* clear incompatible features */
+ if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
+ BdrvCheckResult result;
+ ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
+ if (ret < 0) {
+ return ret;
+ }
This is unnecessary: The image could be opened, so we know that it was
clean when we started. We also know that we haven't crashed yet, so if we
flush all in-memory data, we'll have a consistent on-disk state again.
qcow2_mark_clean() already calls bdrv_flush(bs), so it does everything
that is needed in this respect.
So that flag should always be already cleared at this point anyway?
The qcow2_mark_clean() call is on the next line (which you removed from
the quote), so not at this point but one line later. But yeah, it's done
by other code.
Yes, I was referring to other code (which cleans the image right at
opening).
+ } else if (!strcmp(options[i].name, "encryption")) {
+ if (options[i].value.n != !!s->crypt_method) {
+ fprintf(stderr, "Changing the encryption flag is not "
+ "supported.\n");
+ return -ENOTSUP;
+ }
+ } else if (!strcmp(options[i].name, "cluster_size")) {
+ if (options[i].value.n && (options[i].value.n != s->cluster_size))
{
+ fprintf(stderr, "Changing the cluster size is not "
+ "supported.\n");
+ return -ENOTSUP;
+ }
+ } else if (!strcmp(options[i].name, "lazy_refcounts")) {
+ /* TODO: detect whether this flag was indeed explicitly given */
+ lazy_refcounts = options[i].value.n;
I can see two ways to achieve this:
1. Add a new field 'bool assigned' to QEMUOptionParameter, which would
be cleared before parsing an option string and set for each option in
set_option_parameter()
2. Get the QemuOpts conversion series in and add a function that tells
whether a given option was specified or not.
The same TODO should actually apply to encryption and cluster_size as
well, shouldn't it?
Kind of; however, a cluster_size of 0 seems invalid to me, therefore
it is pretty easy to detect that option not being given.
Depends on whether you think that 'qemu-img amend -o cluster_size=0' is
a valid way of saying that you don't want to change the cluster size. I
would prefer to error out.
No, I just missed the default for that option not being zero. Sorry, my
fault.
Max
