On 12/05/2021 23.10, Kevin Wolf wrote:
> Am 11.05.2021 um 10:35 hat Daniel P. Berrangé geschrieben:
>> On Tue, May 11, 2021 at 01:55:18PM +0800, Li Zhijian wrote:
>>> void bdrv_img_create(const char *filename, const char *fmt,
>>> const char *base_filename, const char *base_fmt,
>>> char *options, uint64_t img_size, int flags, bool
>>> quiet,
>>> @@ -6507,13 +6538,7 @@ void bdrv_img_create(const char *filename, const
>>> char *fmt,
>>>
>>> backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
>>> if (backing_file) {
>>> - if (!strcmp(filename, backing_file)) {
>>> - error_setg(errp, "Error: Trying to create an image with the "
>>> - "same filename as the backing file");
>>> - goto out;
>>> - }
>>> - if (backing_file[0] == '\0') {
>>> - error_setg(errp, "Expected backing file name, got empty
>>> string");
>>> + if (!validate_backing_file(filename, backing_file, errp)) {
>>> goto out;
>>> }
>>> }
>> Thinking about this again, this seems to be quite high in the generic block
>> layer code. As such I don't think we can assume that the backing file here
>> is actually a plain file on disk. IIUC the backing file could still be any
>> of the block drivers. Only once we get down into the protocol specific
>> drivers can be validate the type of backend.
> Yes, you definitely can't assume that filename is really a local file
> name here. It could be any other protocol supported by QEMU, or even use
> the json: pseudo-protocol.
>
>> I'm not sure what the right way to deal with that is, so perhaps Kevin or
>> Max can make a suggestion.
> Can we just keep the backing file open with write permissions unshared
> so that locking will fail for the new image?
*
Not sure if i have understood. In my understanding, open(2) cannot support
'open with write permissions unshared',
it has to cooperate with flock(2)/fcntl(2) to accomplish writing exclusively.
Currently, qemu block also doesn't support 'open with write permissions
unshared', but i found something:
#define BDRV_O_NO_SHARE0x0001 /* don't share permissions */
And I have tried below changes and expect the block fails to write the image.
@@ -6563,7 +6563,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
assert(full_backing);
/* backing files always opened read-only */
- back_flags = flags;
+ back_flags = flags | BDRV_O_NO_SHARE;
back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
backing_options = qdict_new();
But in practice, the image is created successfully.
So do you mean we should implement a new flag like 'BDRV_O_NO_SHARE_WR' to
handle this
*
Thanks
Zhijian
> Or would that error
> condition be detected too late so that the image would already be
> truncated?
>
> Kevin
>
>
>
