[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] raw_bsd: add offset and size options
From: |
Tomáš Golembiovský |
Subject: |
Re: [Qemu-devel] [PATCH v5] raw_bsd: add offset and size options |
Date: |
Tue, 25 Oct 2016 23:37:33 +0200 |
I should test my code more before submitting it to ML. I have found two
bugs in the patch.
On Sun, 23 Oct 2016 16:54:37 +0200
Tomáš Golembiovský <address@hidden> wrote:
> +static int raw_read_options(QDict *options, BlockDriverState *bs,
> + BDRVRawState *s, Error **errp)
> +{
> + Error *local_err = NULL;
> + QemuOpts *opts = NULL;
> + int64_t real_size = 0;
> + int ret;
> +
> + real_size = bdrv_getlength(bs->file->bs);
> + if (real_size < 0) {
> + error_setg_errno(errp, -real_size, "Could not get image size");
> + return real_size;
> + }
> +
> + opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
> + qemu_opts_absorb_qdict(opts, options, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + s->offset = qemu_opt_get_size(opts, "offset", 0);
> + if (qemu_opt_find(opts, "size") != NULL) {
> + s->size = qemu_opt_get_size(opts, "size", 0);
> + s->has_size = true;
> + } else {
> + s->has_size = false;
> + s->size = real_size;
This has to be:
s->size = real_size - s->offset;
.. to account for the offset. Otherwise the following check will fail.
> + }
> +
> + /* Check size and offset */
> + if (real_size < s->offset || (real_size - s->offset) < s->size) {
> + error_setg(errp, "The sum of offset (%" PRIu64 ") and size "
> + "(%" PRIu64 ") has to be smaller or equal to the "
> + " actual size of the containing file (%" PRId64 ")",
> + s->offset, s->size, real_size);
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + /* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
> + * up and leaking out of the specified area. */
> + if (QEMU_IS_ALIGNED(s->size, BDRV_SECTOR_SIZE)) {
The condition has to be negated. Silly mistake made while rewriting the
condition to use QEMU_IS_ALIGNED.
> + error_setg(errp, "Specified size is not multiple of %llu",
> + BDRV_SECTOR_SIZE);
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + ret = 0;
> +
> +end:
> +
> + qemu_opts_del(opts);
> +
> + return ret;
> +}
> +
--
Tomáš Golembiovský <address@hidden>