[PATCH v2 0/4] qsd: Add --daemonize; and add job quit tests

Hanna Reitz posted 4 patches 2 years, 1 month ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220303164814.284974-1-hreitz@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Weil <sw@weilnetz.de>, Hanna Reitz <hreitz@redhat.com>
docs/tools/qemu-storage-daemon.rst   |   7 +
include/sysemu/os-posix.h            |   1 +
include/sysemu/os-win32.h            |   5 +
os-posix.c                           |   6 +
storage-daemon/qemu-storage-daemon.c |  58 +++++++-
tests/qemu-iotests/185               | 190 ++++++++++++++++++++++++++-
tests/qemu-iotests/185.out           |  48 +++++++
7 files changed, 309 insertions(+), 6 deletions(-)
[PATCH v2 0/4] qsd: Add --daemonize; and add job quit tests
Posted by Hanna Reitz 2 years, 1 month ago
Hi,

v1 cover letter:

https://lists.nongnu.org/archive/html/qemu-block/2021-12/msg00499.html


In v2, I followed Vladimir’s suggestion to look into whether we could
reuse os_daemonize().  Indeed we can, and it makes patch 3 (formerly 2)
much simpler!

I decided to leave patch 2 (formerly 1) largely unchanged, because it
seems to me like the point of contention is whether it’s at all
reasonable to introduce a second argument pass for this feature, and not
e.g. which arguments we parse during it.
I believe such an additional pass is a necessity for --daemonize, so
either we really don’t want this pass and so cannot add this feature
(and just drop this series); or we do want this feature, and then we
have to add this pass.


v2:
- Patch 1: Added, so we can use os_daemonize() in patch 3
  (os_daemonize() internally will only do something if the static
  `daemonize` variable is set, which this new os_set_daemonize()
  function does; otherwise, you can only set it by invoking
  os_parse_cmd_args(), which I would rather not (feels like abuse))

- Patch 2:
  - Tried to be more verbose in the commit description
  - Made it clear in process_options() that only QEMU-specific options
    are processed in order

- Patch 3: Vastly simplified by using the existing os_daemonize() and
  os_setup_post() functions


git backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/4:[down] 'os-posix: Add os_set_daemonize()'
002/4:[0006] [FC] 'qsd: Add pre-init argument parsing pass'
003/4:[0148] [FC] 'qsd: Add --daemonize'
004/4:[----] [--] 'iotests/185: Add post-READY quit tests'


Hanna Reitz (4):
  os-posix: Add os_set_daemonize()
  qsd: Add pre-init argument parsing pass
  qsd: Add --daemonize
  iotests/185: Add post-READY quit tests

 docs/tools/qemu-storage-daemon.rst   |   7 +
 include/sysemu/os-posix.h            |   1 +
 include/sysemu/os-win32.h            |   5 +
 os-posix.c                           |   6 +
 storage-daemon/qemu-storage-daemon.c |  58 +++++++-
 tests/qemu-iotests/185               | 190 ++++++++++++++++++++++++++-
 tests/qemu-iotests/185.out           |  48 +++++++
 7 files changed, 309 insertions(+), 6 deletions(-)

-- 
2.34.1


Re: [PATCH v2 0/4] qsd: Add --daemonize; and add job quit tests
Posted by Kevin Wolf 2 years, 1 month ago
Am 03.03.2022 um 17:48 hat Hanna Reitz geschrieben:
> Hi,
> 
> v1 cover letter:
> 
> https://lists.nongnu.org/archive/html/qemu-block/2021-12/msg00499.html
> 
> 
> In v2, I followed Vladimir’s suggestion to look into whether we could
> reuse os_daemonize().  Indeed we can, and it makes patch 3 (formerly 2)
> much simpler!
> 
> I decided to leave patch 2 (formerly 1) largely unchanged, because it
> seems to me like the point of contention is whether it’s at all
> reasonable to introduce a second argument pass for this feature, and not
> e.g. which arguments we parse during it.
> I believe such an additional pass is a necessity for --daemonize, so
> either we really don’t want this pass and so cannot add this feature
> (and just drop this series); or we do want this feature, and then we
> have to add this pass.

Thanks, fixed up as discussed on IRC to address the two minor comments
from Eric and myself, and applied to the block branch.

Kevin