|
From: | Max Reitz |
Subject: | Re: [Qemu-devel] [PATCH v2 7/8] block: Reuse success path from bdrv_open() |
Date: | Wed, 12 Feb 2014 01:26:54 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 10.02.2014 15:56, Kevin Wolf wrote:
Am 08.02.2014 um 18:39 hat Max Reitz geschrieben:The fail and success paths of bdrv_file_open() may be further shortened by reusing code already existent in bdrv_open(). This includes bdrv_file_open() not taking the reference to options which allows the removal of QDECREF(options) in that function. Signed-off-by: Max Reitz <address@hidden> @@ -1001,41 +1003,35 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,/* Parse the filename and open it */if (drv->bdrv_parse_filename && filename) { - drv->bdrv_parse_filename(filename, options, &local_err); + drv->bdrv_parse_filename(filename, *options, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); ret = -EINVAL; goto fail; } - qdict_del(options, "filename"); + qdict_del(*options, "filename"); + } else if (drv->bdrv_needs_filename && !filename) { + error_setg(errp, "The '%s' block driver requires a file name", + drv->format_name); + ret = -EINVAL; + goto fail; }How did this part end up in this patch? It doesn't look wrong, though I think bdrv_open_common() should already catch it. In any case it's an addition that the commit message didn't mention.
I wonder. It definitely doesn't belong here, since as of your commit "block: Fail gracefully with missing filename" this check should be in bdrv_open_common() and not here. I guess, I somehow ended up reverting it to the old state here. I just hope there aren't any more such reverts; I'll take a look. I remember some rebase conflict here (for obvious reasons), so it's probably just in this case, though.
Max
[Prev in Thread] | Current Thread | [Next in Thread] |