Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] scsi: build qemu-pr-helper

[Eric Blake]

On 08/22/2017 08:18 AM, Paolo Bonzini wrote:
> Introduce a privileged helper to run persistent reservation commands.
> This lets virtual machines send persistent reservations without using
> CAP_SYS_RAWIO or out-of-tree patches.  The helper uses Unix permissions
> and SCM_RIGHTS to restrict access to processes that can access its socket
> and prove that they have an open file descriptor for a raw SCSI device.
> 
> The next patch will also correct the usage of persistent reservations
> with multipath devices.
> 
> It would also be possible to support for Linux's IOC_PR_* ioctls in
> the future, to support NVMe devices.  For now, however, only SCSI is
> supported.
> 
> Signed-off-by: Paolo Bonzini 
> ---

> +++ b/docs/interop/pr-helper.rst
> @@ -0,0 +1,78 @@
> +..
> +
> +==
> +Persistent reservation helper protocol
> +==
> +
> +QEMU's SCSI passthrough devices, ``scsi-block`` and ``scsi-generic``,
> +can delegate implementation of persistent reservations to an external
> +(and typically privilege) program.  Persistent Reservations allow

s/privilege/privileged/


> +
> +If a bit is 1 in ``requested_features`` and 0 in ``supported_features``,
> +the corresponding feature is not supported by the helper and the connection
> +is closed.  On the other hand, it is acceptable for a bit to be 0 in
> +``requested_features`` and 1 in ``supported_features``; in this case,
> +he helper will not enable the feature.

s/^he/the/

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] scsi: build qemu-pr-helper

[Paolo Bonzini]

On 22/08/2017 16:34, Marc-André Lureau wrote:
> Could this be handled by udisk? It seems at first the problem is not
> specific to qemu.

Yes, possibly.  In practice, everybody else who uses persistent
reservations seems to run as root. :)

>> +static void usage(const char *name)
>> +{
>> +(printf) (
> Why '(printf)' ? I noticed qemu-nbd use this too.
> 

It lets you use #ifdef inside the argument list.

>>
>> +/* Stash one file descriptor per request.  */
>> +if (nfds) {
>> +for (i = 0; i < nfds; i++) {
>> +if (client->fd == -1) {
>> +client->fd = fds[i++];
>> +} else {
>> +close(fds[i]);
> 
> Isn't this a condition to close the client?

Yes, though I still have to go through the descriptors and close them.

Paolo

Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] scsi: build qemu-pr-helper

[Marc-André Lureau]

Hi

On Tue, Aug 22, 2017 at 3:18 PM, Paolo Bonzini  wrote:
> Introduce a privileged helper to run persistent reservation commands.
> This lets virtual machines send persistent reservations without using
> CAP_SYS_RAWIO or out-of-tree patches.  The helper uses Unix permissions
> and SCM_RIGHTS to restrict access to processes that can access its socket
> and prove that they have an open file descriptor for a raw SCSI device.
>

Could this be handled by udisk? It seems at first the problem is not
specific to qemu.

(I was also wondering if there was any overlap with the stratis
daemon, https://stratis-storage.github.io/, but after 5 minutes
research, probably not, stratis is higher level)

> The next patch will also correct the usage of persistent reservations
> with multipath devices.
>
> It would also be possible to support for Linux's IOC_PR_* ioctls in
> the future, to support NVMe devices.  For now, however, only SCSI is
> supported.
>
> Signed-off-by: Paolo Bonzini 
> ---
>  Makefile   |   9 +-
>  configure  |   2 +-
>  docs/interop/pr-helper.rst |  78 ++
>  docs/pr-manager.rst|  33 +++
>  scsi/pr-helper.h   |  13 +
>  scsi/qemu-pr-helper.c  | 657 
> +
>  6 files changed, 790 insertions(+), 2 deletions(-)
>  create mode 100644 docs/interop/pr-helper.rst
>  create mode 100644 scsi/pr-helper.h
>  create mode 100644 scsi/qemu-pr-helper.c
>
> diff --git a/Makefile b/Makefile
> index 97a58a0f4e..bfd4f69ecd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -209,6 +209,9 @@ ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
>  DOCS+=docs/interop/qemu-qmp-ref.html docs/interop/qemu-qmp-ref.txt 
> docs/interop/qemu-qmp-ref.7
>  DOCS+=docs/interop/qemu-ga-ref.html docs/interop/qemu-ga-ref.txt 
> docs/interop/qemu-ga-ref.7
> +ifdef CONFIG_LINUX
> +DOCS+=scsi/qemu-pr-helper.8
> +endif
>  ifdef CONFIG_VIRTFS
>  DOCS+=fsdev/virtfs-proxy-helper.1
>  endif
> @@ -384,6 +387,8 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o 
> $(COMMON_LDADDS)
>  fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o 
> fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
>  fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
>
> +scsi/qemu-pr-helper$(EXESUF): scsi/qemu-pr-helper.o scsi/utils.o 
> $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
> +
>  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
> $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
> $@,"GEN","$@")
>
> @@ -491,7 +496,7 @@ clean:
> rm -f *.msi
> find . \( -name '*.so' -o -name '*.dll' -o -name '*.mo' -o -name 
> '*.[oda]' \) -type f -exec rm {} +
> rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* 
> *.pod *~ */*~
> -   rm -f fsdev/*.pod
> +   rm -f fsdev/*.pod scsi/*.pod
> rm -f qemu-img-cmds.h
> rm -f ui/shader/*-vert.h ui/shader/*-frag.h
> @# May not be present in GENERATED_FILES
> @@ -590,6 +595,7 @@ endif
>  ifdef CONFIG_VIRTFS
> $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
> $(INSTALL_DATA) fsdev/virtfs-proxy-helper.1 "$(DESTDIR)$(mandir)/man1"
> +   $(INSTALL_DATA) scsi/qemu-pr-helper.8 "$(DESTDIR)$(mandir)/man8"

ifdef CONFIG_LINUX?

>  endif
>
>  install-datadir:
> @@ -718,6 +724,7 @@ qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
> qemu-monitor-info.texi
>  qemu.1: qemu-option-trace.texi
>  qemu-img.1: qemu-img.texi qemu-option-trace.texi qemu-img-cmds.texi
>  fsdev/virtfs-proxy-helper.1: fsdev/virtfs-proxy-helper.texi
> +scsi/qemu-pr-helper.8: scsi/qemu-pr-helper.texi
>  qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
>  qemu-ga.8: qemu-ga.texi
>
> diff --git a/configure b/configure
> index dd73cce62f..772aff18d6 100755
> --- a/configure
> +++ b/configure
> @@ -6545,7 +6545,7 @@ fi
>
>  # build tree in object directory in case the source is not in the current 
> directory
>  DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
> tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
> -DIRS="$DIRS docs docs/interop fsdev"
> +DIRS="$DIRS docs docs/interop fsdev scsi"
>  DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>  DIRS="$DIRS roms/seabios roms/vgabios"
>  DIRS="$DIRS qapi-generated"
> diff --git a/docs/interop/pr-helper.rst b/docs/interop/pr-helper.rst
> new file mode 100644
> index 00..765174c31f
> --- /dev/null
> +++ b/docs/interop/pr-helper.rst
> @@ -0,0 +1,78 @@
> +..
> +
> +==
> +Persistent reservation helper protocol
> +==
> +
> +QEMU's SCSI passthrough devices, ``scsi-block`` and ``scsi-generic``,
> +can delegate implementation of persistent reservations to an external
> +(and typically privilege) program.  Persistent Reservations allow
> +restricting access to block devices to specific initiators in a shared
> +storage setup.
>