diff mbox series

[08/16] block: get file-win32.c handle locking option consistence with file-posix.c

Message ID 20200908194820.702-9-luoyonggang@gmail.com (mailing list archive)
State New, archived
Headers show
Series W32, W64 patches | expand

Commit Message

Yonggang Luo Sept. 8, 2020, 7:48 p.m. UTC
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 block/file-win32.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Kevin Wolf Sept. 9, 2020, 8:19 a.m. UTC | #1
Am 08.09.2020 um 21:48 hat Yonggang Luo geschrieben:
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  block/file-win32.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

This is almost the same as my separately posted 'file-win32: Fix
"locking" option', except that you changed the order of variable
definitions which will cause a merge conflict.

When you take patches from other people, you should correctly attribute
them and not make them look as if they were your own.

Commit messages should not be empty, but explain the motivation for the
change.

In this case, dropping the patch is easiest because my patch is already
in a pull request and will probably be merged soon.

Kevin
Yonggang Luo Sept. 9, 2020, 8:41 a.m. UTC | #2
On Wed, Sep 9, 2020 at 4:19 PM Kevin Wolf <kwolf@redhat.com> wrote:

> Am 08.09.2020 um 21:48 hat Yonggang Luo geschrieben:
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> >  block/file-win32.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
>
> This is almost the same as my separately posted 'file-win32: Fix
> "locking" option', except that you changed the order of variable
> definitions which will cause a merge conflict.
>
> When you take patches from other people, you should correctly attribute
> them and not make them look as if they were your own.
>
> Commit messages should not be empty, but explain the motivation for the
> change.
>
I know someone else have already have fixes for it, so I didn't detail it
much, Ineed
I wanna skip it when sending patches, but  git-publish.py didn't handle it
well under Win32 yet

>
> In this case, dropping the patch is easiest because my patch is already
> in a pull request and will probably be merged soon.
>
I am waiting it to merged, at the current stage, if I don't include this
patch the
test runner will fail.

>
> Kevin
>
>
Kevin Wolf Sept. 9, 2020, 8:57 a.m. UTC | #3
Am 09.09.2020 um 10:41 hat 罗勇刚(Yonggang Luo) geschrieben:
> On Wed, Sep 9, 2020 at 4:19 PM Kevin Wolf <kwolf@redhat.com> wrote:
> 
> > Am 08.09.2020 um 21:48 hat Yonggang Luo geschrieben:
> > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > > ---
> > >  block/file-win32.c | 23 +++++++++++++++++++++--
> > >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > This is almost the same as my separately posted 'file-win32: Fix
> > "locking" option', except that you changed the order of variable
> > definitions which will cause a merge conflict.
> >
> > When you take patches from other people, you should correctly attribute
> > them and not make them look as if they were your own.
> >
> > Commit messages should not be empty, but explain the motivation for the
> > change.
> >
> I know someone else have already have fixes for it, so I didn't detail
> it much, Ineed I wanna skip it when sending patches, but
> git-publish.py didn't handle it well under Win32 yet
> 
> >
> > In this case, dropping the patch is easiest because my patch is already
> > in a pull request and will probably be merged soon.
>
> I am waiting it to merged, at the current stage, if I don't include
> this patch the test runner will fail.

I see. This is not obvious to others and it will look like you want to
get the patch included as you sent it.

What I do in situations like this is that I put any patches from other
series first in my branch and then send out only what is on top of them,
noting the dependency in the cover letter.

Kevin
diff mbox series

Patch

diff --git a/block/file-win32.c b/block/file-win32.c
index ab69bd811a..14e5f5c3b5 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -299,6 +299,11 @@  static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native)",
         },
+        {
+            .name = "locking",
+            .type = QEMU_OPT_STRING,
+            .help = "file locking mode (on/off/auto, default: auto)",
+        },
         { /* end of list */ }
     },
 };
@@ -334,6 +339,7 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     const char *filename;
     bool use_aio;
     int ret;
+    OnOffAuto locking;
 
     s->type = FTYPE_FILE;
 
@@ -342,11 +348,24 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto fail;
     }
-
-    if (qdict_get_try_bool(options, "locking", false)) {
+    locking = qapi_enum_parse(&OnOffAuto_lookup,
+                              qemu_opt_get(opts, "locking"),
+                              ON_OFF_AUTO_AUTO, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+    switch (locking) {
+    case ON_OFF_AUTO_ON:
         error_setg(errp, "locking=on is not supported on Windows");
         ret = -EINVAL;
         goto fail;
+    case ON_OFF_AUTO_OFF:
+    case ON_OFF_AUTO_AUTO:
+        break;
+    default:
+        g_assert_not_reached();
     }
 
     filename = qemu_opt_get(opts, "filename");