On Mon, Jun 6, 2011 at 5:55 PM, Marcelo Tosatti <mtosa...@redhat.com> wrote:
> +/* Valid blkmirror filenames look like
> + * blkmirror:path/to/image1:path/to/image2 */
> +static int blkmirror_open(BlockDriverState *bs, const char *filename, int
> flags)
> +{
> + BdrvMirrorState *m = bs->opaque;
> + int ret, escape, i, n;
> + char *raw;
> +
> + /* Parse the blkmirror: prefix */
> + if (strncmp(filename, "blkmirror:", strlen("blkmirror:"))) {
> + return -EINVAL;
> + }
> + filename += strlen("blkmirror:");
> +
> + /* Parse the raw image filename */
> + raw = malloc(strlen(filename));
Please use qemu_malloc()/qemu_strdup()/qemu_free() instead of the
system library versions.
I'm guilty of this in blkverify :(.
> + escape = 0;
> + for (i = n = 0; i < strlen(filename); i++) {
> + if (!escape && filename[i] == ':') {
> + break;
> + }
> + if (!escape && filename[i] == '\\') {
> + escape = 1;
> + } else {
> + escape = 0;
> + }
> +
> + if (!escape) {
> + raw[n++] = filename[i];
> + }
> + }
> + raw[n] = '\0';
> +
> + m->bs[0] = bdrv_new("");
> + if (m->bs[0] == NULL) {
> + return -ENOMEM;
raw is leaked.
> + }
> + ret = bdrv_open(m->bs[0], raw, flags, NULL);
This isn't necessarily a "raw" file. filename0 and filename1 would be
clearer IMO.
> + free(raw);
> + if (ret < 0) {
> + return ret;
> + }
> + filename += i + 1;
Please document that escaping only takes effect in filename0. After
the second ':' you may no longer escape.
For sanity perhaps the whole string should be unescaped.
> +
> + m->bs[1] = bdrv_new("");
> + if (m->bs[1] == NULL) {
bs[0] is leaked.
> + return -ENOMEM;
> + }
> + ret = bdrv_open(m->bs[1], filename, flags, NULL);
> + if (ret < 0) {
> + bdrv_delete(m->bs[0]);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void blkmirror_close(BlockDriverState *bs)
> +{
> + BdrvMirrorState *m = bs->opaque;
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + bdrv_delete(m->bs[i]);
> + m->bs[i] = NULL;
> + }
> +}
> +
> +static int blkmirror_flush(BlockDriverState *bs)
> +{
> + BdrvMirrorState *m = bs->opaque;
> +
> + bdrv_flush(m->bs[0]);
> + bdrv_flush(m->bs[1]);
Return values should be checked.
Stefan
