qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 1/4] block: introducing 'bdrv_co_delete_file'


From: Daniel Henrique Barboza
Subject: Re: [Qemu-devel] [PATCH v4 1/4] block: introducing 'bdrv_co_delete_file' interface
Date: Tue, 6 Aug 2019 14:02:11 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0



On 8/6/19 12:21 PM, Kevin Wolf wrote:
Am 06.08.2019 um 15:27 hat Daniel Henrique Barboza geschrieben:
diff --git a/block.c b/block.c
index c139540f2b..6e2b0f528d 100644
--- a/block.c
+++ b/block.c
@@ -621,6 +621,17 @@ int get_tmp_filename(char *filename, int size)
   #endif
   }
+/**
+ * Helper that checks if a given string represents a regular
+ * local file.
+ */
+bool bdrv_path_is_regular_file(const char *path)
+{
+    struct stat st;
+
+    return (stat(path, &st) == 0) && S_ISREG(st.st_mode);
+}
+
   /*
    * Detect host devices. By convention, /dev/cdrom[N] is always
    * recognized as a host CDROM.
This hunk isn't generic, it belong in file-posix.c
Patch 3 uses this function as part of the core logic of this fix (do not
delete the file if the file already existed) inside block/cryptoc. This
is the reason it is exposed here. I assumed that we do not want a
public function inside file-posix.c (since there is none - everything
is done using the BD interfaces).
Hm... This doesn't feel right. crypto can't assume that it's working on
a local file. It's working on some lower level BlockDriverState,
whatever that may be. Remember that you could pass all kind of URLs e.g.
for network protocols like NBD or gluster. You don't want to check
whether a local filename exists then.

In fact, I'm not sure if having a special case for an already existing
file is even worth it: By the time we fail, we'll already have truncated
the file, so the old data is lost anyway. Deleting that empty or
half-initialised file doesn't seem much worse than leaving a broken file
behind.

Makes sense. I'll use your argument to justify not handling this 'file
already exists' scenario and simply delete the file in the fail scenario
inside block_crypto_co_create_opts_luks. Then we can move this
path_is_regular_file function inside file-posix.c as you suggested.



Kevin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]