[virt-tools-list] [ [PATCH 1/3] cli: disk: add pr.managed=, pr.type=, pr.path= and pr.mode= support

[Cole Robinson]

On 09/25/2018 05:12 AM, Lin Ma wrote:
> Enable the managed or unmanaged PR configuration to enable SCSI persistent
> reservation for LUN Passthrough.
> 
> Signed-off-by: Lin Ma <lma at suse.com>
> ---
>   man/virt-install.pod                                | 13 +++++++++++++
>   .../compare/virt-install-many-devices.xml           |  9 +++++++++
>   tests/clitest.py                                    |  1 +
>   virtinst/cli.py                                     |  5 +++++
>   virtinst/devices/disk.py                            |  5 +++++
>   5 files changed, 33 insertions(+)
> 
> diff --git a/man/virt-install.pod b/man/virt-install.pod
> index 657ef8cb..abb9d40d 100644
> --- a/man/virt-install.pod
> +++ b/man/virt-install.pod
> @@ -739,6 +739,19 @@ Defines default behavior of the disk during disk snapshots.  See possible
>   values in L<https://www.libvirt.org/formatdomain.html#elementsDisks>,
>   "snapshot" attribute of the <disk> element.
>   
> +=item B<pr.managed, pr.type, pr.path and pr.mode>
> +
> +It enables SCSI persistent reservations for LUN passthrough disks.
> +For possible values, Please refer toPlease refer
> +L<http://www.libvirt.org/formatdomain.html#elementsDisks>,
> +"reservations" attribute of the <source> element.
> +
> +e.g.
> +
> +--disk /dev/sdb,device=lun,bus=scsi,pr.managed=yes
> +
> +--disk /dev/sdc,device=lun,bus=scsi,pr.managed=no,pr.type=unix,pr.path=/tmp/pr-helper0.sock,pr.mode=client
> +
>   =back
>

I don't think this needs to be documented in the manpage. It's poweruser 
enough that users can figure it out on the command line via --disk help 
output. So please drop this bit

For new options I'm trying to get closer to the libvirt XML naming, so 
it's more discoverable and predictable. So I'd like the command line ot be

--disk 
reservations.managed=no,reservations.source.type=unix,reservations.source.path=/tmp/pr-helper0.sock,reservations.source.mode=client

>   See the examples section for some uses. This option deprecates -f/--file,
> diff --git a/tests/cli-test-xml/compare/virt-install-many-devices.xml b/tests/cli-test-xml/compare/virt-install-many-devices.xml
> index 25070b1b..20071439 100644
> --- a/tests/cli-test-xml/compare/virt-install-many-devices.xml
> +++ b/tests/cli-test-xml/compare/virt-install-many-devices.xml
> @@ -160,6 +160,15 @@
>         <source file="/tmp/brand-new.img"/>
>         <target dev="vdn" bus="virtio"/>
>       </disk>
> +    <disk type="block" device="lun">
> +      <driver name="qemu" type="raw"/>
> +      <source dev="/dev/sda">
> +        <reservations managed="no">
> +          <source type="unix" path="/var/run/test/pr-helper0.sock" mode="client"/>
> +        </reservations>
> +      </source>
> +      <target dev="sdd" bus="scsi"/>
> +    </disk>
>       <controller type="usb" index="0" model="ich9-ehci1">
>         <address type="pci" domain="0" bus="0" slot="4" function="7"/>
>       </controller>
> diff --git a/tests/clitest.py b/tests/clitest.py
> index 04795e05..47e2b6dc 100644
> --- a/tests/clitest.py
> +++ b/tests/clitest.py
> @@ -438,6 +438,7 @@ c.add_compare(""" \
>   --disk /var,device=floppy,address.type=ccw,address.cssid=0xfe,address.ssid=0,address.devno=01 \
>   --disk %(NEWIMG2)s,size=1,backing_store=/tmp/foo.img,backing_format=vmdk \
>   --disk /tmp/brand-new.img,size=1,backing_store=/dev/default-pool/iso-vol \
> +--disk path=/dev/sda,device=lun,bus=scsi,pr.managed=no,pr.type=unix,pr.path=/var/run/test/pr-helper0.sock,pr.mode=client \
>   \
>   --network user,mac=12:34:56:78:11:22,portgroup=foo,link_state=down,rom_bar=on,rom_file=/tmp/foo \
>   --network bridge=foobar,model=virtio,driver_name=qemu,driver_queues=3 \
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index d7cb3ac3..b2adbcd2 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -2130,6 +2130,11 @@ ParserDisk.add_arg("geometry_heads", "geometry.heads")
>   ParserDisk.add_arg("geometry_secs", "geometry.secs")
>   ParserDisk.add_arg("geometry_trans", "geometry.trans")
>   
> +ParserDisk.add_arg("pr_managed", "pr.managed")
> +ParserDisk.add_arg("pr_type", "pr.type")
> +ParserDisk.add_arg("pr_path", "pr.path")
> +ParserDisk.add_arg("pr_mode", "pr.mode")
> +

>   #####################
>   # --network parsing #
> diff --git a/virtinst/devices/disk.py b/virtinst/devices/disk.py
> index d3ec27f9..8b125e16 100644
> --- a/virtinst/devices/disk.py
> +++ b/virtinst/devices/disk.py
> @@ -836,6 +836,11 @@ class DeviceDisk(Device):
>       geometry_secs = XMLProperty("./geometry/@secs", is_int=True)
>       geometry_trans = XMLProperty("./geometry/@trans")
>   
> +    pr_managed = XMLProperty("./source/reservations/@managed")
> +    pr_type = XMLProperty("./source/reservations/source/@type")
> +    pr_path = XMLProperty("./source/reservations/source/@path")
> +    pr_mode = XMLProperty("./source/reservations/source/@mode")
> +

Similarly rename these properties reservation_X and reservation_source_X

Patch looks good otherwise.

Thanks,
Cole