[PATCH 00/16] virsh: Add support for network disks to --attach-disk

older

Availability of physical devices...

Plans for the next release

Peter Krempa

Thursday, 19 November 2020 Thu, 19 Nov '20

10:26 a.m.

...

<source dev='asdfsource'/> <target dev='tgt'/> </disk> I've rather rewrote how we do things than trying to investigate why the original version [1] failed. The new version also adds tests so it's simpler to see that things actually work. [1] https://www.redhat.com/archives/libvir-list/2020-November/msg01071.html Peter Krempa (15): tests: virsh: Add simple testsuite for using --print-xml of various commands virsh: cmdAttachDisk: Declare one variable per line virsh: cmdAttachDisk: Use automatic memory clearing for 'xml' and 'dom' virsh: cmdAttachDisk: Refactor control flow virsh: cmdAttachDisk: Don't assume type='block' if file can't be examined virsh: cmdAttachDisk: Split out formatting of disk address virsh: Extract address formatting from cmdAttachDiskFormatAddress virsh: Unify code for <address type='drive' virsh: Refactor str2DiskAddress virsh: Rename 'struct DiskAddress' and friends virsh: virshAddress*: Move out of cmdAttachDisk block virsh: cmdAttachDisk: Use enum for disk source type virsh: cmdAttachDisk: Use virXMLFormatElement tests: virsh-output: Mark as expensive tests: virsh-checkpoint/snapshot: Mark as expensive Ryan Gahagan (1): virsh: Added attach-disk support for network disk docs/manpages/virsh.rst | 31 +- tests/meson.build | 1 + tests/virsh-checkpoint | 2 + tests/virsh-output | 29 ++ tests/virsh-output-commands | 94 +++++ tests/virsh-output.out | 496 ++++++++++++++++++++++++ tests/virsh-snapshot | 2 + tools/virsh-domain.c | 748 ++++++++++++++++++------------------ 8 files changed, 1017 insertions(+), 386 deletions(-) create mode 100755 tests/virsh-output create mode 100755 tests/virsh-output-commands create mode 100644 tests/virsh-output.out -- 2.28.0

Show replies by date

Peter Krempa

Thursday, 19 November Thu, 19 Nov

10:26 a.m.

New subject: [PATCH 01/16] tests: virsh: Add simple testsuite for using --print-xml of various commands

The test uses a script and compares the output against a template file VIR_TEST_REGENREATE_OUTPUT can be used on test failures. This test will be marked as expensive once the refactors it guards are done. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/meson.build | 1 + tests/virsh-output | 27 +++ tests/virsh-output-commands | 80 +++++++ tests/virsh-output.out | 408 ++++++++++++++++++++++++++++++++++++ 4 files changed, 516 insertions(+) create mode 100755 tests/virsh-output create mode 100755 tests/virsh-output-commands create mode 100644 tests/virsh-output.out diff --git a/tests/meson.build b/tests/meson.build index f88410ff33..3ef4d06177 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -654,6 +654,7 @@ if conf.has('WITH_LIBVIRTD') 'virsh-define-dev-segfault', 'virsh-int-overflow', 'virsh-optparse', + 'virsh-output', 'virsh-read-bufsiz', 'virsh-read-non-seekable', 'virsh-schedinfo', diff --git a/tests/virsh-output b/tests/virsh-output new file mode 100755 index 0000000000..44833c7048 --- /dev/null +++ b/tests/virsh-output @@ -0,0 +1,27 @@ +#!/bin/sh + +. "$(dirname $0)/test-lib.sh" + +if [ ! -x /bin/bash ]; then + echo skipping test - no bash + exit 77 +fi + +mock_xdg_ || framework_failure + +export LIBVIRT_DEFAULT_URI='test:///default' +ln -s $abs_top_builddir/tools/virsh virsh + +$abs_top_srcdir/tests/virsh-output-commands > out 2>&1 + +echo end >> out + +if compare $abs_top_srcdir/tests/virsh-output.out out; then + exit 0 +else + if [ "x$VIR_TEST_REGENERATE_OUTPUT" = "x1" ]; then + cp out $abs_top_srcdir/tests/virsh-output.out + fi + + exit 1 +fi diff --git a/tests/virsh-output-commands b/tests/virsh-output-commands new file mode 100755 index 0000000000..43f87bee0f --- /dev/null +++ b/tests/virsh-output-commands @@ -0,0 +1,80 @@ +#!/bin/bash + +attach_disk() +{ + ./virsh attach-disk --print-xml --domain testdom $@ --source /nonexistent/file +} + +set -x + +./virsh attach-disk +./virsh attach-disk --print-xml --domain testdom $@ --source "" +./virsh attach-disk --print-xml --domain testdom $@ --source "" --sourcetype file +./virsh attach-disk --print-xml --domain testdom $@ --source "" --sourcetype blah +attach_disk --target hda +attach_disk --target hda --sourcetype file +attach_disk --target hda --sourcetype block +attach_disk --target hda --sourcetype nothing +attach_disk --target hda --sourcetype file --type disk +attach_disk --target hda --sourcetype block --type disk +attach_disk --target hda --sourcetype file --type cdrom +attach_disk --target hda --sourcetype block --type cdrom +attach_disk --target hda --sourcetype file --type blah +attach_disk --target hda --sourcetype block --type blah +attach_disk --target hda --sourcetype file --type disk --driver testdriver +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 +attach_disk --target hda --sourcetype file --type disk --subdriver raw +attach_disk --target hda --sourcetype file --type disk --cache none +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --cache none +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL +attach_disk --target hda --sourcetype file --type disk --mode readonly +attach_disk --target hda --sourcetype file --type disk --mode shareable +attach_disk --target hda --sourcetype file --type disk --mode whatever +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias +attach_disk --target hda --sourcetype file --type disk --rawio +attach_disk --target hda --sourcetype file --type disk --multifunction +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu + +attach_disk --target hda --sourcetype file --type disk --address ide:1.2.3 +attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4 +attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5 +attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address ide:1.2.3 + +attach_disk --target sda --sourcetype file --type disk --address usb:12.34 +attach_disk --target sda --sourcetype file --type disk --address usb:12.3 +attach_disk --target sda --sourcetype file --type disk --address usb:12:34 +attach_disk --target vda --sourcetype file --type disk --address usb:12.34 +attach_disk --target hda --sourcetype file --type disk --address usb:12.34 + +attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.3 +attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4 +attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5 +attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address scsi:1.2.3 + +attach_disk --target sda --sourcetype file --type disk --address sata:1.2.3 +attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4 +attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5 +attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address sata:1.2.3 + +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 +attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa +attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa +attach_disk --target sda --sourcetype file --type disk --address pci:12.34.56.aa + +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction +attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction + +attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56 +attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target sda --sourcetype file --type disk --address ccw:12.34.56 + +attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 +attach_disk --target vda --sourcetype file --type disk --address test:12:34:56 +attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 diff --git a/tests/virsh-output.out b/tests/virsh-output.out new file mode 100644 index 0000000000..8fbe02a232 --- /dev/null +++ b/tests/virsh-output.out @@ -0,0 +1,408 @@ ++ ./virsh attach-disk +error: command 'attach-disk' requires <domain> option +error: command 'attach-disk' requires <source> option +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' --sourcetype file +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' --sourcetype blah +error: command 'attach-disk' requires <target> option ++ attach_disk --target hda ++ ./virsh attach-disk --print-xml --domain testdom --target hda --source /nonexistent/file +<disk type='block'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --source /nonexistent/file +<disk type='file'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --source /nonexistent/file +<disk type='block'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype nothing ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype nothing --source /nonexistent/file +error: Unknown source type: 'nothing' + ++ attach_disk --target hda --sourcetype file --type disk ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type disk ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type disk --source /nonexistent/file +<disk type='block' device='disk'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type cdrom ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type cdrom --source /nonexistent/file +<disk type='file' device='cdrom'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type cdrom ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type cdrom --source /nonexistent/file +<disk type='block' device='cdrom'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type blah ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type blah --source /nonexistent/file +<disk type='file' device='blah'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type blah ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type blah --source /nonexistent/file +<disk type='block' device='blah'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --driver testdriver ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --driver testdriver --source /nonexistent/file +<disk type='file' device='disk'> + <driver name='testdriver'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver raw ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver raw --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='raw'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --cache none ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --cache none --source /nonexistent/file +<disk type='file' device='disk'> + <driver cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --cache none ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --cache none --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2' cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <serial>TEST_SERIAL</serial> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode readonly ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode readonly --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <readonly/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode shareable ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode shareable --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <shareable/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode whatever ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode whatever --source /nonexistent/file +error: No support for whatever in command 'attach-disk' + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <alias name='testalias'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --rawio ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --rawio --source /nonexistent/file +<disk type='file' device='disk' rawio='yes'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu --source /nonexistent/file +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' iothread='3' cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <readonly/> + <alias name='testalias'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='34' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12:34 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12:34 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='34' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target hda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.78 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12:34:56:78 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12:34:56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12.34.56 --source /nonexistent/file +error: Invalid address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12:34:56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12:34:56 --source /nonexistent/file +error: Invalid address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12.34.56 --source /nonexistent/file +error: Invalid address. + +end -- 2.28.0

Daniel Henrique Barboza

Friday, 20 November Fri, 20 Nov

5:47 a.m.

New subject: [PATCH 01/16] tests: virsh: Add simple testsuite for using --print-xml of various commands

On 11/19/20 1:26 PM, Peter Krempa wrote:

...

The test uses a script and compares the output against a template file VIR_TEST_REGENREATE_OUTPUT can be used on test failures. This test will

A typo and ... a missing period perhaps? "... against a template file. VIR_TEST_REGENERATE_OUTPUT can be used ..." Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>

...

be marked as expensive once the refactors it guards are done. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/meson.build | 1 + tests/virsh-output | 27 +++ tests/virsh-output-commands | 80 +++++++ tests/virsh-output.out | 408 ++++++++++++++++++++++++++++++++++++ 4 files changed, 516 insertions(+) create mode 100755 tests/virsh-output create mode 100755 tests/virsh-output-commands create mode 100644 tests/virsh-output.out diff --git a/tests/meson.build b/tests/meson.build index f88410ff33..3ef4d06177 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -654,6 +654,7 @@ if conf.has('WITH_LIBVIRTD') 'virsh-define-dev-segfault', 'virsh-int-overflow', 'virsh-optparse', + 'virsh-output', 'virsh-read-bufsiz', 'virsh-read-non-seekable', 'virsh-schedinfo', diff --git a/tests/virsh-output b/tests/virsh-output new file mode 100755 index 0000000000..44833c7048 --- /dev/null +++ b/tests/virsh-output @@ -0,0 +1,27 @@ +#!/bin/sh + +. "$(dirname $0)/test-lib.sh" + +if [ ! -x /bin/bash ]; then + echo skipping test - no bash + exit 77 +fi + +mock_xdg_ || framework_failure + +export LIBVIRT_DEFAULT_URI='test:///default' +ln -s $abs_top_builddir/tools/virsh virsh + +$abs_top_srcdir/tests/virsh-output-commands > out 2>&1 + +echo end >> out + +if compare $abs_top_srcdir/tests/virsh-output.out out; then + exit 0 +else + if [ "x$VIR_TEST_REGENERATE_OUTPUT" = "x1" ]; then + cp out $abs_top_srcdir/tests/virsh-output.out + fi + + exit 1 +fi diff --git a/tests/virsh-output-commands b/tests/virsh-output-commands new file mode 100755 index 0000000000..43f87bee0f --- /dev/null +++ b/tests/virsh-output-commands @@ -0,0 +1,80 @@ +#!/bin/bash + +attach_disk() +{ + ./virsh attach-disk --print-xml --domain testdom $@ --source /nonexistent/file +} + +set -x + +./virsh attach-disk +./virsh attach-disk --print-xml --domain testdom $@ --source "" +./virsh attach-disk --print-xml --domain testdom $@ --source "" --sourcetype file +./virsh attach-disk --print-xml --domain testdom $@ --source "" --sourcetype blah +attach_disk --target hda +attach_disk --target hda --sourcetype file +attach_disk --target hda --sourcetype block +attach_disk --target hda --sourcetype nothing +attach_disk --target hda --sourcetype file --type disk +attach_disk --target hda --sourcetype block --type disk +attach_disk --target hda --sourcetype file --type cdrom +attach_disk --target hda --sourcetype block --type cdrom +attach_disk --target hda --sourcetype file --type blah +attach_disk --target hda --sourcetype block --type blah +attach_disk --target hda --sourcetype file --type disk --driver testdriver +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 +attach_disk --target hda --sourcetype file --type disk --subdriver raw +attach_disk --target hda --sourcetype file --type disk --cache none +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --cache none +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL +attach_disk --target hda --sourcetype file --type disk --mode readonly +attach_disk --target hda --sourcetype file --type disk --mode shareable +attach_disk --target hda --sourcetype file --type disk --mode whatever +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias +attach_disk --target hda --sourcetype file --type disk --rawio +attach_disk --target hda --sourcetype file --type disk --multifunction +attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu + +attach_disk --target hda --sourcetype file --type disk --address ide:1.2.3 +attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4 +attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5 +attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address ide:1.2.3 + +attach_disk --target sda --sourcetype file --type disk --address usb:12.34 +attach_disk --target sda --sourcetype file --type disk --address usb:12.3 +attach_disk --target sda --sourcetype file --type disk --address usb:12:34 +attach_disk --target vda --sourcetype file --type disk --address usb:12.34 +attach_disk --target hda --sourcetype file --type disk --address usb:12.34 + +attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.3 +attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4 +attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5 +attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address scsi:1.2.3 + +attach_disk --target sda --sourcetype file --type disk --address sata:1.2.3 +attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4 +attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5 +attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3 +attach_disk --target vda --sourcetype file --type disk --address sata:1.2.3 + +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 +attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa +attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa +attach_disk --target sda --sourcetype file --type disk --address pci:12.34.56.aa + +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction +attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction +attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction + +attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56 +attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56 +attach_disk --target sda --sourcetype file --type disk --address ccw:12.34.56 + +attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 +attach_disk --target vda --sourcetype file --type disk --address test:12:34:56 +attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 diff --git a/tests/virsh-output.out b/tests/virsh-output.out new file mode 100644 index 0000000000..8fbe02a232 --- /dev/null +++ b/tests/virsh-output.out @@ -0,0 +1,408 @@ ++ ./virsh attach-disk +error: command 'attach-disk' requires <domain> option +error: command 'attach-disk' requires <source> option +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' --sourcetype file +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' --sourcetype blah +error: command 'attach-disk' requires <target> option ++ attach_disk --target hda ++ ./virsh attach-disk --print-xml --domain testdom --target hda --source /nonexistent/file +<disk type='block'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --source /nonexistent/file +<disk type='file'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --source /nonexistent/file +<disk type='block'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype nothing ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype nothing --source /nonexistent/file +error: Unknown source type: 'nothing' + ++ attach_disk --target hda --sourcetype file --type disk ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type disk ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type disk --source /nonexistent/file +<disk type='block' device='disk'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type cdrom ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type cdrom --source /nonexistent/file +<disk type='file' device='cdrom'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type cdrom ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type cdrom --source /nonexistent/file +<disk type='block' device='cdrom'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type blah ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type blah --source /nonexistent/file +<disk type='file' device='blah'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype block --type blah ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype block --type blah --source /nonexistent/file +<disk type='block' device='blah'> + <source dev='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --driver testdriver ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --driver testdriver --source /nonexistent/file +<disk type='file' device='disk'> + <driver name='testdriver'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver raw ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver raw --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='raw'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --cache none ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --cache none --source /nonexistent/file +<disk type='file' device='disk'> + <driver cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --cache none ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --cache none --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2' cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --serial TEST_SERIAL --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <serial>TEST_SERIAL</serial> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode readonly ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode readonly --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <readonly/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode shareable ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode shareable --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <shareable/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --mode whatever ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --mode whatever --source /nonexistent/file +error: No support for whatever in command 'attach-disk' + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --source /nonexistent/file +<disk type='file' device='disk'> + <driver type='qcow2'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <alias name='testalias'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --rawio ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --rawio --source /nonexistent/file +<disk type='file' device='disk' rawio='yes'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --subdriver qcow2 --alias testalias --iothread 3 --mode readonly --cache none --driver qemu --source /nonexistent/file +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' iothread='3' cache='none'/> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <readonly/> + <alias name='testalias'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ide:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='hda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address ide:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ide:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='34' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address usb:12:34 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address usb:12:34 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='usb' bus='12' port='34' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target hda --sourcetype file --type disk --address usb:12.34 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address usb:12.34 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address scsi:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address scsi:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address scsi:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='3' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1.2.4 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='4' /> +</disk> + ++ attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address sata:1:2:5 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='sda'/> + <address type='drive' controller='1' bus='2' unit='5' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address sata:1.2.3 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address sata:1.2.3 --source /nonexistent/file +error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.78 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12:34:56:78 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa'/> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address pci:12.34.56.aa ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address pci:12.34.56.aa --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.78 --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa' multifunction='on'/> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12:34:56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +<disk type='file' device='disk'> + <source file='/nonexistent/file'/> + <target dev='vda'/> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> +</disk> + ++ attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +error: expecting an ide:00.00.00 address. + ++ attach_disk --target sda --sourcetype file --type disk --address ccw:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target sda --sourcetype file --type disk --address ccw:12.34.56 --source /nonexistent/file +error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12.34.56 --source /nonexistent/file +error: Invalid address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12:34:56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12:34:56 --source /nonexistent/file +error: Invalid address. + ++ attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 ++ ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12.34.56 --source /nonexistent/file +error: Invalid address. + +end

Peter Krempa

Thursday, 19 November Thu, 19 Nov

10:26 a.m.

New subject: [PATCH 02/16] virsh: cmdAttachDisk: Declare one variable per line

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c999458d72..e27b779533 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -562,13 +562,23 @@ static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - const char *source = NULL, *target = NULL, *driver = NULL, - *subdriver = NULL, *type = NULL, *mode = NULL, - *iothread = NULL, *cache = NULL, *io = NULL, - *serial = NULL, *straddr = NULL, *wwn = NULL, - *targetbus = NULL, *alias = NULL; + const char *source = NULL; + const char *target = NULL; + const char *driver = NULL; + const char *subdriver = NULL; + const char *type = NULL; + const char *mode = NULL; + const char *iothread = NULL; + const char *cache = NULL; + const char *io = NULL; + const char *serial = NULL; + const char *straddr = NULL; + const char *wwn = NULL; + const char *targetbus = NULL; + const char *alias = NULL; struct DiskAddress diskAddr; - bool isFile = false, functionReturn = false; + bool isFile = false; + bool functionReturn = false; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 03/16] virsh: cmdAttachDisk: Use automatic memory clearing for 'xml' and 'dom'

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e27b779533..42a255cd6d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -561,7 +561,7 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { - virDomainPtr dom = NULL; + g_autoptr(virshDomain) dom = NULL; const char *source = NULL; const char *target = NULL; const char *driver = NULL; @@ -583,7 +583,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *xml = NULL; + g_autofree char *xml = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); bool config = vshCommandOptBool(cmd, "config"); @@ -783,8 +783,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } cleanup: - VIR_FREE(xml); - virshDomainFree(dom); return functionReturn; } -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 04/16] virsh: cmdAttachDisk: Refactor control flow

Remove the unnecessary 'cleanup:' label since we can directly return as the memory clearing is now automated. We can also remove the 'functionReturn' variable and use the usual pattern of returning success. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 42a255cd6d..2eaac9ad8c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -578,7 +578,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *alias = NULL; struct DiskAddress diskAddr; bool isFile = false; - bool functionReturn = false; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; @@ -615,7 +614,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) - goto cleanup; + return false; if (!stype) { if (driver && (STREQ(driver, "file") || STREQ(driver, "tap"))) { @@ -628,19 +627,19 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) isFile = true; } else if (STRNEQ(stype, "block")) { vshError(ctl, _("Unknown source type: '%s'"), stype); - goto cleanup; + return false; } if (mode) { if (STRNEQ(mode, "readonly") && STRNEQ(mode, "shareable")) { vshError(ctl, _("No support for %s in command 'attach-disk'"), mode); - goto cleanup; + return false; } } if (wwn && !virValidateWWN(wwn)) - goto cleanup; + return false; /* Make XML of disk */ virBufferAsprintf(&buf, "<disk type='%s'", @@ -692,7 +691,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (straddr) { if (str2DiskAddress(straddr, &diskAddr) != 0) { vshError(ctl, _("Invalid address.")); - goto cleanup; + return false; } if (STRPREFIX((const char *)target, "vd")) { @@ -714,7 +713,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.")); - goto cleanup; + return false; } } else if (STRPREFIX((const char *)target, "sd")) { if (diskAddr.type == DISK_ADDR_TYPE_SCSI) { @@ -736,7 +735,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, "%s", _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.")); - goto cleanup; + return false; } } else if (STRPREFIX((const char *)target, "hd")) { if (diskAddr.type == DISK_ADDR_TYPE_IDE) { @@ -747,7 +746,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) diskAddr.addr.ide.unit); } else { vshError(ctl, "%s", _("expecting an ide:00.00.00 address.")); - goto cleanup; + return false; } } } @@ -759,12 +758,11 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "print-xml")) { vshPrint(ctl, "%s", xml); - functionReturn = true; - goto cleanup; + return true; } if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (persistent && virDomainIsActive(dom) == 1) @@ -775,15 +773,14 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) else ret = virDomainAttachDevice(dom, xml); - if (ret != 0) { + if (ret < 0) { vshError(ctl, "%s", _("Failed to attach disk")); - } else { - vshPrintExtra(ctl, "%s", _("Disk attached successfully\n")); - functionReturn = true; + return false; } - cleanup: - return functionReturn; + + vshPrintExtra(ctl, "%s", _("Disk attached successfully\n")); + return true; } /* -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 05/16] virsh: cmdAttachDisk: Don't assume type='block' if file can't be examined

'virsh attach-disk' uses stat() to determine if the 'source' is a regular file. If stat fails though it assumes that the file is block. Since it's way more common to have regular files and the detection does not work at all when accessing a remote host, modify the default to assume type='file' by default. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/virsh-output.out | 4 ++-- tools/virsh-domain.c | 30 +++++++++++++++++------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/virsh-output.out b/tests/virsh-output.out index 8fbe02a232..a93492e8f8 100644 --- a/tests/virsh-output.out +++ b/tests/virsh-output.out @@ -10,8 +10,8 @@ error: command 'attach-disk' requires <target> option error: command 'attach-disk' requires <target> option + attach_disk --target hda + ./virsh attach-disk --print-xml --domain testdom --target hda --source /nonexistent/file -<disk type='block'> - <source dev='/nonexistent/file'/> +<disk type='file'> + <source file='/nonexistent/file'/> <target dev='hda'/> </disk> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2eaac9ad8c..18e9ab0b46 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -577,7 +577,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *targetbus = NULL; const char *alias = NULL; struct DiskAddress diskAddr; - bool isFile = false; + bool isBlock = false; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; @@ -617,15 +617,15 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) return false; if (!stype) { - if (driver && (STREQ(driver, "file") || STREQ(driver, "tap"))) { - isFile = true; - } else { - if (source && !stat(source, &st)) - isFile = S_ISREG(st.st_mode) ? true : false; - } - } else if (STREQ(stype, "file")) { - isFile = true; - } else if (STRNEQ(stype, "block")) { + if (STRNEQ_NULLABLE(driver, "file") && + STRNEQ_NULLABLE(driver, "tap") && + source && + stat(source, &st) == 0 && + S_ISBLK(st.st_mode)) + isBlock = true; + } else if (STREQ(stype, "block")) { + isBlock = true; + } else if (STRNEQ(stype, "file")) { vshError(ctl, _("Unknown source type: '%s'"), stype); return false; } @@ -642,8 +642,12 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) return false; /* Make XML of disk */ - virBufferAsprintf(&buf, "<disk type='%s'", - isFile ? "file" : "block"); + virBufferAddLit(&buf, "<disk"); + if (isBlock) + virBufferAddLit(&buf, " type='block'"); + else + virBufferAddLit(&buf, " type='file'"); + if (type) virBufferAsprintf(&buf, " device='%s'", type); if (vshCommandOptBool(cmd, "rawio")) @@ -670,7 +674,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (source) virBufferAsprintf(&buf, "<source %s='%s'/>\n", - isFile ? "file" : "dev", source); + !isBlock ? "file" : "dev", source); virBufferAsprintf(&buf, "<target dev='%s'", target); if (targetbus) virBufferAsprintf(&buf, " bus='%s'", targetbus); -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 06/16] virsh: cmdAttachDisk: Split out formatting of disk address

First step is to remove all of the address handling code to a new function called 'cmdAttachDiskFormatAddress'. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 142 ++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 63 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 18e9ab0b46..84b8a6fb52 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -558,6 +558,81 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) return -1; } + +static int +cmdAttachDiskFormatAddress(vshControl *ctl, + virBufferPtr buf, + const char *straddr, + const char *target, + bool multifunction) +{ + struct DiskAddress diskAddr; + + if (str2DiskAddress(straddr, &diskAddr) != 0) { + vshError(ctl, _("Invalid address.")); + return -1; + } + + if (STRPREFIX((const char *)target, "vd")) { + if (diskAddr.type == DISK_ADDR_TYPE_PCI) { + virBufferAsprintf(buf, + "<address type='pci' domain='0x%04x'" + " bus ='0x%02x' slot='0x%02x' function='0x%0x'", + diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, + diskAddr.addr.pci.slot, diskAddr.addr.pci.function); + if (multifunction) + virBufferAddLit(buf, " multifunction='on'"); + virBufferAddLit(buf, "/>\n"); + } else if (diskAddr.type == DISK_ADDR_TYPE_CCW) { + virBufferAsprintf(buf, + "<address type='ccw' cssid='0x%02x'" + " ssid='0x%01x' devno='0x%04x' />\n", + diskAddr.addr.ccw.cssid, diskAddr.addr.ccw.ssid, + diskAddr.addr.ccw.devno); + } else { + vshError(ctl, "%s", + _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.")); + return -1; + } + } else if (STRPREFIX((const char *)target, "sd")) { + if (diskAddr.type == DISK_ADDR_TYPE_SCSI) { + virBufferAsprintf(buf, + "<address type='drive' controller='%u'" + " bus='%u' unit='%llu' />\n", + diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus, + diskAddr.addr.scsi.unit); + } else if (diskAddr.type == DISK_ADDR_TYPE_USB) { + virBufferAsprintf(buf, + "<address type='usb' bus='%u' port='%u' />\n", + diskAddr.addr.usb.bus, diskAddr.addr.usb.port); + } else if (diskAddr.type == DISK_ADDR_TYPE_SATA) { + virBufferAsprintf(buf, + "<address type='drive' controller='%u'" + " bus='%u' unit='%llu' />\n", + diskAddr.addr.sata.controller, diskAddr.addr.sata.bus, + diskAddr.addr.sata.unit); + } else { + vshError(ctl, "%s", + _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.")); + return -1; + } + } else if (STRPREFIX((const char *)target, "hd")) { + if (diskAddr.type == DISK_ADDR_TYPE_IDE) { + virBufferAsprintf(buf, + "<address type='drive' controller='%u'" + " bus='%u' unit='%u' />\n", + diskAddr.addr.ide.controller, diskAddr.addr.ide.bus, + diskAddr.addr.ide.unit); + } else { + vshError(ctl, "%s", _("expecting an ide:00.00.00 address.")); + return -1; + } + } + + return 0; +} + + static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { @@ -576,7 +651,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *wwn = NULL; const char *targetbus = NULL; const char *alias = NULL; - struct DiskAddress diskAddr; bool isBlock = false; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; @@ -588,6 +662,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool persistent = vshCommandOptBool(cmd, "persistent"); + bool multifunction = vshCommandOptBool(cmd, "multifunction"); VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); @@ -692,68 +767,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (wwn) virBufferAsprintf(&buf, "<wwn>%s</wwn>\n", wwn); - if (straddr) { - if (str2DiskAddress(straddr, &diskAddr) != 0) { - vshError(ctl, _("Invalid address.")); - return false; - } - - if (STRPREFIX((const char *)target, "vd")) { - if (diskAddr.type == DISK_ADDR_TYPE_PCI) { - virBufferAsprintf(&buf, - "<address type='pci' domain='0x%04x'" - " bus ='0x%02x' slot='0x%02x' function='0x%0x'", - diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, - diskAddr.addr.pci.slot, diskAddr.addr.pci.function); - if (vshCommandOptBool(cmd, "multifunction")) - virBufferAddLit(&buf, " multifunction='on'"); - virBufferAddLit(&buf, "/>\n"); - } else if (diskAddr.type == DISK_ADDR_TYPE_CCW) { - virBufferAsprintf(&buf, - "<address type='ccw' cssid='0x%02x'" - " ssid='0x%01x' devno='0x%04x' />\n", - diskAddr.addr.ccw.cssid, diskAddr.addr.ccw.ssid, - diskAddr.addr.ccw.devno); - } else { - vshError(ctl, "%s", - _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.")); - return false; - } - } else if (STRPREFIX((const char *)target, "sd")) { - if (diskAddr.type == DISK_ADDR_TYPE_SCSI) { - virBufferAsprintf(&buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%llu' />\n", - diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus, - diskAddr.addr.scsi.unit); - } else if (diskAddr.type == DISK_ADDR_TYPE_USB) { - virBufferAsprintf(&buf, - "<address type='usb' bus='%u' port='%u' />\n", - diskAddr.addr.usb.bus, diskAddr.addr.usb.port); - } else if (diskAddr.type == DISK_ADDR_TYPE_SATA) { - virBufferAsprintf(&buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%llu' />\n", - diskAddr.addr.sata.controller, diskAddr.addr.sata.bus, - diskAddr.addr.sata.unit); - } else { - vshError(ctl, "%s", - _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.")); - return false; - } - } else if (STRPREFIX((const char *)target, "hd")) { - if (diskAddr.type == DISK_ADDR_TYPE_IDE) { - virBufferAsprintf(&buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%u' />\n", - diskAddr.addr.ide.controller, diskAddr.addr.ide.bus, - diskAddr.addr.ide.unit); - } else { - vshError(ctl, "%s", _("expecting an ide:00.00.00 address.")); - return false; - } - } - } + if (straddr && + cmdAttachDiskFormatAddress(ctl, &buf, straddr, target, multifunction) < 0) + return false; virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</disk>\n"); -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 07/16] virsh: Extract address formatting from cmdAttachDiskFormatAddress

Introduce virshAddressFormat with code from cmdAttachDiskFormatAddress to format the address. Note that this patch fixes some whitespace inconsistencies in the formatted addresses. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/virsh-output.out | 42 +++++++------- tools/virsh-domain.c | 122 ++++++++++++++++++++++++++--------------- 2 files changed, 99 insertions(+), 65 deletions(-) diff --git a/tests/virsh-output.out b/tests/virsh-output.out index a93492e8f8..6af7600a9c 100644 --- a/tests/virsh-output.out +++ b/tests/virsh-output.out @@ -182,7 +182,7 @@ error: No support for whatever in command 'attach-disk' <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='hda'/> - <address type='drive' controller='1' bus='2' unit='3' /> + <address type='drive' controller='1' bus='2' unit='3'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address ide:1.2.4 @@ -190,7 +190,7 @@ error: No support for whatever in command 'attach-disk' <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='hda'/> - <address type='drive' controller='1' bus='2' unit='4' /> + <address type='drive' controller='1' bus='2' unit='4'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address ide:1:2:5 @@ -198,7 +198,7 @@ error: No support for whatever in command 'attach-disk' <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='hda'/> - <address type='drive' controller='1' bus='2' unit='5' /> + <address type='drive' controller='1' bus='2' unit='5'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address ide:1.2.3 @@ -214,7 +214,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='usb' bus='12' port='34' /> + <address type='usb' bus='12' port='34'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address usb:12.3 @@ -222,7 +222,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='usb' bus='12' port='3' /> + <address type='usb' bus='12' port='3'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address usb:12:34 @@ -230,7 +230,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='usb' bus='12' port='34' /> + <address type='usb' bus='12' port='34'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address usb:12.34 @@ -246,7 +246,7 @@ error: expecting an ide:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='3' /> + <address type='drive' controller='1' bus='2' unit='3'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address scsi:1.2.4 @@ -254,7 +254,7 @@ error: expecting an ide:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='4' /> + <address type='drive' controller='1' bus='2' unit='4'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address scsi:1:2:5 @@ -262,7 +262,7 @@ error: expecting an ide:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='5' /> + <address type='drive' controller='1' bus='2' unit='5'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address scsi:1.2.3 @@ -278,7 +278,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='3' /> + <address type='drive' controller='1' bus='2' unit='3'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address sata:1.2.4 @@ -286,7 +286,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='4' /> + <address type='drive' controller='1' bus='2' unit='4'/> </disk> + attach_disk --target sda --sourcetype file --type disk --address sata:1:2:5 @@ -294,7 +294,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='sda'/> - <address type='drive' controller='1' bus='2' unit='5' /> + <address type='drive' controller='1' bus='2' unit='5'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address sata:1.2.3 @@ -310,7 +310,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 @@ -318,7 +318,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa @@ -326,7 +326,7 @@ error: expecting a pci:0000.00.00.00 or ccw:00.0.0000 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0xaa'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address pci:12.34.56.aa @@ -342,7 +342,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78' multifunction='on'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address pci:12:34:56:78 --multifunction @@ -350,7 +350,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0x78' multifunction='on'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0x78' multifunction='on'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address pci:12.34.56.aa --multifunction @@ -358,7 +358,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='pci' domain='0x0012' bus ='0x34' slot='0x56' function='0xaa' multifunction='on'/> + <address type='pci' domain='0x0012' bus='0x34' slot='0x56' function='0xaa' multifunction='on'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 @@ -366,7 +366,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address ccw:12:34:56 @@ -374,7 +374,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/> </disk> + attach_disk --target vda --sourcetype file --type disk --address ccw:12.34.56 @@ -382,7 +382,7 @@ error: expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address. <disk type='file' device='disk'> <source file='/nonexistent/file'/> <target dev='vda'/> - <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056' /> + <address type='ccw' cssid='0x12' ssid='0x34' devno='0x0056'/> </disk> + attach_disk --target hda --sourcetype file --type disk --address ccw:12.34.56 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 84b8a6fb52..e1a0b9087b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -305,7 +305,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = NULL} }; -enum { +enum diskAddrType { DISK_ADDR_TYPE_INVALID, DISK_ADDR_TYPE_PCI, DISK_ADDR_TYPE_SCSI, @@ -320,6 +320,7 @@ struct PCIAddress { unsigned int bus; unsigned int slot; unsigned int function; + bool multifunction; }; struct SCSIAddress { @@ -521,7 +522,7 @@ static int str2SATAAddress(const char *str, struct SATAAddress *sataAddr) * sata disk address: sata:00.00.0 (controller:bus:unit) */ -static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) +static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr, bool multifunction) { char *type, *addr; @@ -537,6 +538,7 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) if (STREQLEN(type, "pci", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_PCI; + diskAddr->addr.pci.multifunction = multifunction; return str2PCIAddress(addr + 1, &diskAddr->addr.pci); } else if (STREQLEN(type, "scsi", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_SCSI; @@ -559,6 +561,71 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) } +static void +virshAddressFormat(virBufferPtr buf, + struct DiskAddress *addr) +{ + switch ((enum diskAddrType) addr->type) { + case DISK_ADDR_TYPE_PCI: + virBufferAsprintf(buf, + "<address type='pci' domain='0x%04x' bus='0x%02x' slot='0x%02x' function='0x%0x'", + addr->addr.pci.domain, + addr->addr.pci.bus, + addr->addr.pci.slot, + addr->addr.pci.function); + + if (addr->addr.pci.multifunction) + virBufferAddLit(buf, " multifunction='on'"); + + virBufferAddLit(buf, "/>\n"); + break; + + case DISK_ADDR_TYPE_SCSI: + virBufferAsprintf(buf, + "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", + addr->addr.scsi.controller, + addr->addr.scsi.bus, + addr->addr.scsi.unit); + break; + + case DISK_ADDR_TYPE_IDE: + virBufferAsprintf(buf, + "<address type='drive' controller='%u' bus='%u' unit='%u'/>\n", + addr->addr.ide.controller, + addr->addr.ide.bus, + addr->addr.ide.unit); + break; + + case DISK_ADDR_TYPE_CCW: + virBufferAsprintf(buf, + "<address type='ccw' cssid='0x%02x' ssid='0x%01x' devno='0x%04x'/>\n", + addr->addr.ccw.cssid, + addr->addr.ccw.ssid, + addr->addr.ccw.devno); + break; + + case DISK_ADDR_TYPE_USB: + virBufferAsprintf(buf, + "<address type='usb' bus='%u' port='%u'/>\n", + addr->addr.usb.bus, + addr->addr.usb.port); + break; + + case DISK_ADDR_TYPE_SATA: + virBufferAsprintf(buf, + "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", + addr->addr.sata.controller, + addr->addr.sata.bus, + addr->addr.sata.unit); + break; + + case DISK_ADDR_TYPE_INVALID: + default: + return; + } +} + + static int cmdAttachDiskFormatAddress(vshControl *ctl, virBufferPtr buf, @@ -568,67 +635,34 @@ cmdAttachDiskFormatAddress(vshControl *ctl, { struct DiskAddress diskAddr; - if (str2DiskAddress(straddr, &diskAddr) != 0) { + if (str2DiskAddress(straddr, &diskAddr, multifunction) != 0) { vshError(ctl, _("Invalid address.")); return -1; } if (STRPREFIX((const char *)target, "vd")) { - if (diskAddr.type == DISK_ADDR_TYPE_PCI) { - virBufferAsprintf(buf, - "<address type='pci' domain='0x%04x'" - " bus ='0x%02x' slot='0x%02x' function='0x%0x'", - diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, - diskAddr.addr.pci.slot, diskAddr.addr.pci.function); - if (multifunction) - virBufferAddLit(buf, " multifunction='on'"); - virBufferAddLit(buf, "/>\n"); - } else if (diskAddr.type == DISK_ADDR_TYPE_CCW) { - virBufferAsprintf(buf, - "<address type='ccw' cssid='0x%02x'" - " ssid='0x%01x' devno='0x%04x' />\n", - diskAddr.addr.ccw.cssid, diskAddr.addr.ccw.ssid, - diskAddr.addr.ccw.devno); - } else { + if (diskAddr.type != DISK_ADDR_TYPE_PCI && + diskAddr.type != DISK_ADDR_TYPE_CCW) { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.")); return -1; } } else if (STRPREFIX((const char *)target, "sd")) { - if (diskAddr.type == DISK_ADDR_TYPE_SCSI) { - virBufferAsprintf(buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%llu' />\n", - diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus, - diskAddr.addr.scsi.unit); - } else if (diskAddr.type == DISK_ADDR_TYPE_USB) { - virBufferAsprintf(buf, - "<address type='usb' bus='%u' port='%u' />\n", - diskAddr.addr.usb.bus, diskAddr.addr.usb.port); - } else if (diskAddr.type == DISK_ADDR_TYPE_SATA) { - virBufferAsprintf(buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%llu' />\n", - diskAddr.addr.sata.controller, diskAddr.addr.sata.bus, - diskAddr.addr.sata.unit); - } else { + if (diskAddr.type != DISK_ADDR_TYPE_SCSI && + diskAddr.type != DISK_ADDR_TYPE_USB && + diskAddr.type != DISK_ADDR_TYPE_SATA) { vshError(ctl, "%s", _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.")); return -1; } } else if (STRPREFIX((const char *)target, "hd")) { - if (diskAddr.type == DISK_ADDR_TYPE_IDE) { - virBufferAsprintf(buf, - "<address type='drive' controller='%u'" - " bus='%u' unit='%u' />\n", - diskAddr.addr.ide.controller, diskAddr.addr.ide.bus, - diskAddr.addr.ide.unit); - } else { + if (diskAddr.type != DISK_ADDR_TYPE_IDE) { vshError(ctl, "%s", _("expecting an ide:00.00.00 address.")); return -1; } } + virshAddressFormat(buf, &diskAddr); return 0; } @@ -1006,7 +1040,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: { - struct PCIAddress pciAddr = {0, 0, 0, 0}; + struct PCIAddress pciAddr = {0, 0, 0, 0, false}; if (str2PCIAddress(source, &pciAddr) < 0) { vshError(ctl, _("cannot parse pci address '%s' for network " -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 08/16] virsh: Unify code for <address type='drive'

DISK_ADDR_TYPE_SATA, DISK_ADDR_TYPE_IDE and DISK_ADDR_TYPE_SCSI are driven by basically identical data types. Unify them. Note that changes to 'str2DiskAddress' are deliberately lazy as it will be refactored later. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 100 +++++-------------------------------------- 1 file changed, 11 insertions(+), 89 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e1a0b9087b..942974fd39 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -323,18 +323,12 @@ struct PCIAddress { bool multifunction; }; -struct SCSIAddress { +struct DriveAddress { unsigned int controller; unsigned int bus; unsigned long long unit; }; -struct IDEAddress { - unsigned int controller; - unsigned int bus; - unsigned int unit; -}; - struct CCWAddress { unsigned int cssid; unsigned int ssid; @@ -346,21 +340,13 @@ struct USBAddress { unsigned int port; }; -struct SATAAddress { - unsigned int controller; - unsigned int bus; - unsigned long long unit; -}; - struct DiskAddress { int type; union { struct PCIAddress pci; - struct SCSIAddress scsi; - struct IDEAddress ide; + struct DriveAddress drive; struct CCWAddress ccw; struct USBAddress usb; - struct SATAAddress sata; } addr; }; @@ -393,7 +379,7 @@ static int str2PCIAddress(const char *str, struct PCIAddress *pciAddr) return 0; } -static int str2SCSIAddress(const char *str, struct SCSIAddress *scsiAddr) +static int str2DriveAddress(const char *str, struct DriveAddress *scsiAddr) { char *controller, *bus, *unit; @@ -418,31 +404,6 @@ static int str2SCSIAddress(const char *str, struct SCSIAddress *scsiAddr) return 0; } -static int str2IDEAddress(const char *str, struct IDEAddress *ideAddr) -{ - char *controller, *bus, *unit; - - if (!ideAddr) - return -1; - if (!str) - return -1; - - controller = (char *)str; - - if (virStrToLong_uip(controller, &bus, 10, &ideAddr->controller) != 0) - return -1; - - bus++; - if (virStrToLong_uip(bus, &unit, 10, &ideAddr->bus) != 0) - return -1; - - unit++; - if (virStrToLong_uip(unit, NULL, 10, &ideAddr->unit) != 0) - return -1; - - return 0; -} - static int str2CCWAddress(const char *str, struct CCWAddress *ccwAddr) { char *cssid, *ssid, *devno; @@ -489,31 +450,6 @@ static int str2USBAddress(const char *str, struct USBAddress *usbAddr) return 0; } -static int str2SATAAddress(const char *str, struct SATAAddress *sataAddr) -{ - char *controller, *bus, *unit; - - if (!sataAddr) - return -1; - if (!str) - return -1; - - controller = (char *)str; - - if (virStrToLong_uip(controller, &bus, 10, &sataAddr->controller) != 0) - return -1; - - bus++; - if (virStrToLong_uip(bus, &unit, 10, &sataAddr->bus) != 0) - return -1; - - unit++; - if (virStrToLong_ullp(unit, NULL, 10, &sataAddr->unit) != 0) - return -1; - - return 0; -} - /* pci address pci:0000.00.0x0a.0 (domain:bus:slot:function) * ide disk address: ide:00.00.0 (controller:bus:unit) * scsi disk address: scsi:00.00.0 (controller:bus:unit) @@ -542,10 +478,10 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr, bool m return str2PCIAddress(addr + 1, &diskAddr->addr.pci); } else if (STREQLEN(type, "scsi", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_SCSI; - return str2SCSIAddress(addr + 1, &diskAddr->addr.scsi); + return str2DriveAddress(addr + 1, &diskAddr->addr.drive); } else if (STREQLEN(type, "ide", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_IDE; - return str2IDEAddress(addr + 1, &diskAddr->addr.ide); + return str2DriveAddress(addr + 1, &diskAddr->addr.drive); } else if (STREQLEN(type, "ccw", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_CCW; return str2CCWAddress(addr + 1, &diskAddr->addr.ccw); @@ -554,7 +490,7 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr, bool m return str2USBAddress(addr + 1, &diskAddr->addr.usb); } else if (STREQLEN(type, "sata", addr - type)) { diskAddr->type = DISK_ADDR_TYPE_SATA; - return str2SATAAddress(addr + 1, &diskAddr->addr.sata); + return str2DriveAddress(addr + 1, &diskAddr->addr.drive); } return -1; @@ -580,20 +516,14 @@ virshAddressFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); break; + case DISK_ADDR_TYPE_SATA: + case DISK_ADDR_TYPE_IDE: case DISK_ADDR_TYPE_SCSI: virBufferAsprintf(buf, "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", - addr->addr.scsi.controller, - addr->addr.scsi.bus, - addr->addr.scsi.unit); - break; - - case DISK_ADDR_TYPE_IDE: - virBufferAsprintf(buf, - "<address type='drive' controller='%u' bus='%u' unit='%u'/>\n", - addr->addr.ide.controller, - addr->addr.ide.bus, - addr->addr.ide.unit); + addr->addr.drive.controller, + addr->addr.drive.bus, + addr->addr.drive.unit); break; case DISK_ADDR_TYPE_CCW: @@ -611,14 +541,6 @@ virshAddressFormat(virBufferPtr buf, addr->addr.usb.port); break; - case DISK_ADDR_TYPE_SATA: - virBufferAsprintf(buf, - "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", - addr->addr.sata.controller, - addr->addr.sata.bus, - addr->addr.sata.unit); - break; - case DISK_ADDR_TYPE_INVALID: default: return; -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 09/16] virsh: Refactor str2DiskAddress

Rewrite and rename the address parser. As a fallout the use of the removed 'str2PCIAddress' is replaced by virshAddressParse and virshAddressFormat. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 207 ++++++++++++++----------------------------- 1 file changed, 67 insertions(+), 140 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 942974fd39..57b17f3b5c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -306,15 +306,26 @@ static const vshCmdOptDef opts_attach_disk[] = { }; enum diskAddrType { - DISK_ADDR_TYPE_INVALID, DISK_ADDR_TYPE_PCI, DISK_ADDR_TYPE_SCSI, DISK_ADDR_TYPE_IDE, DISK_ADDR_TYPE_CCW, DISK_ADDR_TYPE_USB, DISK_ADDR_TYPE_SATA, + + DISK_ADDR_TYPE_LAST }; +VIR_ENUM_DECL(diskAddr); +VIR_ENUM_IMPL(diskAddr, + DISK_ADDR_TYPE_LAST, + "pci", + "scsi", + "ide", + "ccw", + "usb", + "sata"); + struct PCIAddress { unsigned int domain; unsigned int bus; @@ -350,105 +361,6 @@ struct DiskAddress { } addr; }; -static int str2PCIAddress(const char *str, struct PCIAddress *pciAddr) -{ - char *domain, *bus, *slot, *function; - - if (!pciAddr) - return -1; - if (!str) - return -1; - - domain = (char *)str; - - if (virStrToLong_uip(domain, &bus, 16, &pciAddr->domain) != 0) - return -1; - - bus++; - if (virStrToLong_uip(bus, &slot, 16, &pciAddr->bus) != 0) - return -1; - - slot++; - if (virStrToLong_uip(slot, &function, 16, &pciAddr->slot) != 0) - return -1; - - function++; - if (virStrToLong_uip(function, NULL, 16, &pciAddr->function) != 0) - return -1; - - return 0; -} - -static int str2DriveAddress(const char *str, struct DriveAddress *scsiAddr) -{ - char *controller, *bus, *unit; - - if (!scsiAddr) - return -1; - if (!str) - return -1; - - controller = (char *)str; - - if (virStrToLong_uip(controller, &bus, 10, &scsiAddr->controller) != 0) - return -1; - - bus++; - if (virStrToLong_uip(bus, &unit, 10, &scsiAddr->bus) != 0) - return -1; - - unit++; - if (virStrToLong_ullp(unit, NULL, 10, &scsiAddr->unit) != 0) - return -1; - - return 0; -} - -static int str2CCWAddress(const char *str, struct CCWAddress *ccwAddr) -{ - char *cssid, *ssid, *devno; - - if (!ccwAddr) - return -1; - if (!str) - return -1; - - cssid = (char *)str; - - if (virStrToLong_uip(cssid, &ssid, 16, &ccwAddr->cssid) != 0) - return -1; - - ssid++; - if (virStrToLong_uip(ssid, &devno, 16, &ccwAddr->ssid) != 0) - return -1; - - devno++; - if (virStrToLong_uip(devno, NULL, 16, &ccwAddr->devno) != 0) - return -1; - - return 0; -} - -static int str2USBAddress(const char *str, struct USBAddress *usbAddr) -{ - char *bus, *port; - - if (!usbAddr) - return -1; - if (!str) - return -1; - - bus = (char *)str; - - if (virStrToLong_uip(bus, &port, 10, &usbAddr->bus) != 0) - return -1; - - port++; - if (virStrToLong_uip(port, NULL, 10, &usbAddr->port) != 0) - return -1; - - return 0; -} /* pci address pci:0000.00.0x0a.0 (domain:bus:slot:function) * ide disk address: ide:00.00.0 (controller:bus:unit) @@ -457,43 +369,60 @@ static int str2USBAddress(const char *str, struct USBAddress *usbAddr) * usb disk address: usb:00.00 (bus:port) * sata disk address: sata:00.00.0 (controller:bus:unit) */ - -static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr, bool multifunction) +static int +virshAddressParse(const char *str, + bool multifunction, + struct DiskAddress *addr) { - char *type, *addr; + g_autofree char *type = g_strdup(str); + char *a = strchr(type, ':'); - if (!diskAddr) - return -1; - if (!str) - return -1; - - type = (char *)str; - addr = strchr(type, ':'); if (!addr) return -1; - if (STREQLEN(type, "pci", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_PCI; - diskAddr->addr.pci.multifunction = multifunction; - return str2PCIAddress(addr + 1, &diskAddr->addr.pci); - } else if (STREQLEN(type, "scsi", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_SCSI; - return str2DriveAddress(addr + 1, &diskAddr->addr.drive); - } else if (STREQLEN(type, "ide", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_IDE; - return str2DriveAddress(addr + 1, &diskAddr->addr.drive); - } else if (STREQLEN(type, "ccw", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_CCW; - return str2CCWAddress(addr + 1, &diskAddr->addr.ccw); - } else if (STREQLEN(type, "usb", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_USB; - return str2USBAddress(addr + 1, &diskAddr->addr.usb); - } else if (STREQLEN(type, "sata", addr - type)) { - diskAddr->type = DISK_ADDR_TYPE_SATA; - return str2DriveAddress(addr + 1, &diskAddr->addr.drive); + *a = '\0'; + + addr->type = diskAddrTypeFromString(type); + + switch ((enum diskAddrType) addr->type) { + case DISK_ADDR_TYPE_PCI: + addr->addr.pci.multifunction = multifunction; + + if (virStrToLong_uip(++a, &a, 16, &addr->addr.pci.domain) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.bus) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.slot) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.function) < 0) + return -1; + break; + + case DISK_ADDR_TYPE_SATA: + case DISK_ADDR_TYPE_IDE: + case DISK_ADDR_TYPE_SCSI: + if (virStrToLong_uip(++a, &a, 10, &addr->addr.drive.controller) < 0 || + virStrToLong_uip(++a, &a, 10, &addr->addr.drive.bus) < 0 || + virStrToLong_ullp(++a, &a, 10, &addr->addr.drive.unit) < 0) + return -1; + break; + + case DISK_ADDR_TYPE_CCW: + if (virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.cssid) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.ssid) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.devno) < 0) + return -1; + break; + + case DISK_ADDR_TYPE_USB: + if (virStrToLong_uip(++a, &a, 10, &addr->addr.usb.bus) < 0 || + virStrToLong_uip(++a, &a, 10, &addr->addr.usb.port) < 0) + return -1; + break; + + case DISK_ADDR_TYPE_LAST: + default: + return -1; } - return -1; + return 0; } @@ -541,7 +470,7 @@ virshAddressFormat(virBufferPtr buf, addr->addr.usb.port); break; - case DISK_ADDR_TYPE_INVALID: + case DISK_ADDR_TYPE_LAST: default: return; } @@ -557,7 +486,7 @@ cmdAttachDiskFormatAddress(vshControl *ctl, { struct DiskAddress diskAddr; - if (str2DiskAddress(straddr, &diskAddr, multifunction) != 0) { + if (virshAddressParse(straddr, multifunction, &diskAddr) < 0) { vshError(ctl, _("Invalid address.")); return -1; } @@ -962,20 +891,18 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: { - struct PCIAddress pciAddr = {0, 0, 0, 0, false}; + g_autofree char *pciaddrstr = g_strdup_printf("pci:%s", source); + struct DiskAddress addr = { 0 }; - if (str2PCIAddress(source, &pciAddr) < 0) { - vshError(ctl, _("cannot parse pci address '%s' for network " - "interface"), source); + if (virshAddressParse(pciaddrstr, false, &addr) < 0) { + vshError(ctl, _("cannot parse pci address '%s' for network interface"), + source); goto cleanup; } virBufferAddLit(&buf, "<source>\n"); virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, "<address type='pci' domain='0x%04x'" - " bus='0x%02x' slot='0x%02x' function='0x%d'/>\n", - pciAddr.domain, pciAddr.bus, - pciAddr.slot, pciAddr.function); + virshAddressFormat(&buf, &addr); virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</source>\n"); break; -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 10/16] virsh: Rename 'struct DiskAddress' and friends

Use 'virshAddress' prefix for all the related structs and enums. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 96 ++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 57b17f3b5c..afb859eaa0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -305,20 +305,20 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = NULL} }; -enum diskAddrType { - DISK_ADDR_TYPE_PCI, - DISK_ADDR_TYPE_SCSI, - DISK_ADDR_TYPE_IDE, - DISK_ADDR_TYPE_CCW, - DISK_ADDR_TYPE_USB, - DISK_ADDR_TYPE_SATA, +enum virshAddressType { + VIRSH_ADDRESS_TYPE_PCI, + VIRSH_ADDRESS_TYPE_SCSI, + VIRSH_ADDRESS_TYPE_IDE, + VIRSH_ADDRESS_TYPE_CCW, + VIRSH_ADDRESS_TYPE_USB, + VIRSH_ADDRESS_TYPE_SATA, - DISK_ADDR_TYPE_LAST + VIRSH_ADDRESS_TYPE_LAST }; -VIR_ENUM_DECL(diskAddr); -VIR_ENUM_IMPL(diskAddr, - DISK_ADDR_TYPE_LAST, +VIR_ENUM_DECL(virshAddress); +VIR_ENUM_IMPL(virshAddress, + VIRSH_ADDRESS_TYPE_LAST, "pci", "scsi", "ide", @@ -326,7 +326,7 @@ VIR_ENUM_IMPL(diskAddr, "usb", "sata"); -struct PCIAddress { +struct virshAddressPCI { unsigned int domain; unsigned int bus; unsigned int slot; @@ -334,30 +334,30 @@ struct PCIAddress { bool multifunction; }; -struct DriveAddress { +struct virshAddressDrive { unsigned int controller; unsigned int bus; unsigned long long unit; }; -struct CCWAddress { +struct virshAddressCCW { unsigned int cssid; unsigned int ssid; unsigned int devno; }; -struct USBAddress { +struct virshAddressUSB { unsigned int bus; unsigned int port; }; -struct DiskAddress { - int type; +struct virshAddress { + int type; /* enum virshAddressType */ union { - struct PCIAddress pci; - struct DriveAddress drive; - struct CCWAddress ccw; - struct USBAddress usb; + struct virshAddressPCI pci; + struct virshAddressDrive drive; + struct virshAddressCCW ccw; + struct virshAddressUSB usb; } addr; }; @@ -372,7 +372,7 @@ struct DiskAddress { static int virshAddressParse(const char *str, bool multifunction, - struct DiskAddress *addr) + struct virshAddress *addr) { g_autofree char *type = g_strdup(str); char *a = strchr(type, ':'); @@ -382,10 +382,10 @@ virshAddressParse(const char *str, *a = '\0'; - addr->type = diskAddrTypeFromString(type); + addr->type = virshAddressTypeFromString(type); - switch ((enum diskAddrType) addr->type) { - case DISK_ADDR_TYPE_PCI: + switch ((enum virshAddressType) addr->type) { + case VIRSH_ADDRESS_TYPE_PCI: addr->addr.pci.multifunction = multifunction; if (virStrToLong_uip(++a, &a, 16, &addr->addr.pci.domain) < 0 || @@ -395,29 +395,29 @@ virshAddressParse(const char *str, return -1; break; - case DISK_ADDR_TYPE_SATA: - case DISK_ADDR_TYPE_IDE: - case DISK_ADDR_TYPE_SCSI: + case VIRSH_ADDRESS_TYPE_SATA: + case VIRSH_ADDRESS_TYPE_IDE: + case VIRSH_ADDRESS_TYPE_SCSI: if (virStrToLong_uip(++a, &a, 10, &addr->addr.drive.controller) < 0 || virStrToLong_uip(++a, &a, 10, &addr->addr.drive.bus) < 0 || virStrToLong_ullp(++a, &a, 10, &addr->addr.drive.unit) < 0) return -1; break; - case DISK_ADDR_TYPE_CCW: + case VIRSH_ADDRESS_TYPE_CCW: if (virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.cssid) < 0 || virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.ssid) < 0 || virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.devno) < 0) return -1; break; - case DISK_ADDR_TYPE_USB: + case VIRSH_ADDRESS_TYPE_USB: if (virStrToLong_uip(++a, &a, 10, &addr->addr.usb.bus) < 0 || virStrToLong_uip(++a, &a, 10, &addr->addr.usb.port) < 0) return -1; break; - case DISK_ADDR_TYPE_LAST: + case VIRSH_ADDRESS_TYPE_LAST: default: return -1; } @@ -428,10 +428,10 @@ virshAddressParse(const char *str, static void virshAddressFormat(virBufferPtr buf, - struct DiskAddress *addr) + struct virshAddress *addr) { - switch ((enum diskAddrType) addr->type) { - case DISK_ADDR_TYPE_PCI: + switch ((enum virshAddressType) addr->type) { + case VIRSH_ADDRESS_TYPE_PCI: virBufferAsprintf(buf, "<address type='pci' domain='0x%04x' bus='0x%02x' slot='0x%02x' function='0x%0x'", addr->addr.pci.domain, @@ -445,9 +445,9 @@ virshAddressFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); break; - case DISK_ADDR_TYPE_SATA: - case DISK_ADDR_TYPE_IDE: - case DISK_ADDR_TYPE_SCSI: + case VIRSH_ADDRESS_TYPE_SATA: + case VIRSH_ADDRESS_TYPE_IDE: + case VIRSH_ADDRESS_TYPE_SCSI: virBufferAsprintf(buf, "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", addr->addr.drive.controller, @@ -455,7 +455,7 @@ virshAddressFormat(virBufferPtr buf, addr->addr.drive.unit); break; - case DISK_ADDR_TYPE_CCW: + case VIRSH_ADDRESS_TYPE_CCW: virBufferAsprintf(buf, "<address type='ccw' cssid='0x%02x' ssid='0x%01x' devno='0x%04x'/>\n", addr->addr.ccw.cssid, @@ -463,14 +463,14 @@ virshAddressFormat(virBufferPtr buf, addr->addr.ccw.devno); break; - case DISK_ADDR_TYPE_USB: + case VIRSH_ADDRESS_TYPE_USB: virBufferAsprintf(buf, "<address type='usb' bus='%u' port='%u'/>\n", addr->addr.usb.bus, addr->addr.usb.port); break; - case DISK_ADDR_TYPE_LAST: + case VIRSH_ADDRESS_TYPE_LAST: default: return; } @@ -484,7 +484,7 @@ cmdAttachDiskFormatAddress(vshControl *ctl, const char *target, bool multifunction) { - struct DiskAddress diskAddr; + struct virshAddress diskAddr; if (virshAddressParse(straddr, multifunction, &diskAddr) < 0) { vshError(ctl, _("Invalid address.")); @@ -492,22 +492,22 @@ cmdAttachDiskFormatAddress(vshControl *ctl, } if (STRPREFIX((const char *)target, "vd")) { - if (diskAddr.type != DISK_ADDR_TYPE_PCI && - diskAddr.type != DISK_ADDR_TYPE_CCW) { + if (diskAddr.type != VIRSH_ADDRESS_TYPE_PCI && + diskAddr.type != VIRSH_ADDRESS_TYPE_CCW) { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 or ccw:00.0.0000 address.")); return -1; } } else if (STRPREFIX((const char *)target, "sd")) { - if (diskAddr.type != DISK_ADDR_TYPE_SCSI && - diskAddr.type != DISK_ADDR_TYPE_USB && - diskAddr.type != DISK_ADDR_TYPE_SATA) { + if (diskAddr.type != VIRSH_ADDRESS_TYPE_SCSI && + diskAddr.type != VIRSH_ADDRESS_TYPE_USB && + diskAddr.type != VIRSH_ADDRESS_TYPE_SATA) { vshError(ctl, "%s", _("expecting a scsi:00.00.00 or usb:00.00 or sata:00.00.00 address.")); return -1; } } else if (STRPREFIX((const char *)target, "hd")) { - if (diskAddr.type != DISK_ADDR_TYPE_IDE) { + if (diskAddr.type != VIRSH_ADDRESS_TYPE_IDE) { vshError(ctl, "%s", _("expecting an ide:00.00.00 address.")); return -1; } @@ -892,7 +892,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_HOSTDEV: { g_autofree char *pciaddrstr = g_strdup_printf("pci:%s", source); - struct DiskAddress addr = { 0 }; + struct virshAddress addr = { 0 }; if (virshAddressParse(pciaddrstr, false, &addr) < 0) { vshError(ctl, _("cannot parse pci address '%s' for network interface"), -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 11/16] virsh: virshAddress*: Move out of cmdAttachDisk block

The helper started as helper for cmdAttachDisk but is now used outside of it too. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 343 ++++++++++++++++++++++--------------------- 1 file changed, 172 insertions(+), 171 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index afb859eaa0..f8d97593b8 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -52,6 +52,178 @@ #include "virenum.h" #include "virutil.h" +enum virshAddressType { + VIRSH_ADDRESS_TYPE_PCI, + VIRSH_ADDRESS_TYPE_SCSI, + VIRSH_ADDRESS_TYPE_IDE, + VIRSH_ADDRESS_TYPE_CCW, + VIRSH_ADDRESS_TYPE_USB, + VIRSH_ADDRESS_TYPE_SATA, + + VIRSH_ADDRESS_TYPE_LAST +}; + +VIR_ENUM_DECL(virshAddress); +VIR_ENUM_IMPL(virshAddress, + VIRSH_ADDRESS_TYPE_LAST, + "pci", + "scsi", + "ide", + "ccw", + "usb", + "sata"); + +struct virshAddressPCI { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; + bool multifunction; +}; + +struct virshAddressDrive { + unsigned int controller; + unsigned int bus; + unsigned long long unit; +}; + +struct virshAddressCCW { + unsigned int cssid; + unsigned int ssid; + unsigned int devno; +}; + +struct virshAddressUSB { + unsigned int bus; + unsigned int port; +}; + +struct virshAddress { + int type; /* enum virshAddressType */ + union { + struct virshAddressPCI pci; + struct virshAddressDrive drive; + struct virshAddressCCW ccw; + struct virshAddressUSB usb; + } addr; +}; + + +/* pci address pci:0000.00.0x0a.0 (domain:bus:slot:function) + * ide disk address: ide:00.00.0 (controller:bus:unit) + * scsi disk address: scsi:00.00.0 (controller:bus:unit) + * ccw disk address: ccw:0xfe.0.0000 (cssid:ssid:devno) + * usb disk address: usb:00.00 (bus:port) + * sata disk address: sata:00.00.0 (controller:bus:unit) + */ +static int +virshAddressParse(const char *str, + bool multifunction, + struct virshAddress *addr) +{ + g_autofree char *type = g_strdup(str); + char *a = strchr(type, ':'); + + if (!addr) + return -1; + + *a = '\0'; + + addr->type = virshAddressTypeFromString(type); + + switch ((enum virshAddressType) addr->type) { + case VIRSH_ADDRESS_TYPE_PCI: + addr->addr.pci.multifunction = multifunction; + + if (virStrToLong_uip(++a, &a, 16, &addr->addr.pci.domain) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.bus) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.slot) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.pci.function) < 0) + return -1; + break; + + case VIRSH_ADDRESS_TYPE_SATA: + case VIRSH_ADDRESS_TYPE_IDE: + case VIRSH_ADDRESS_TYPE_SCSI: + if (virStrToLong_uip(++a, &a, 10, &addr->addr.drive.controller) < 0 || + virStrToLong_uip(++a, &a, 10, &addr->addr.drive.bus) < 0 || + virStrToLong_ullp(++a, &a, 10, &addr->addr.drive.unit) < 0) + return -1; + break; + + case VIRSH_ADDRESS_TYPE_CCW: + if (virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.cssid) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.ssid) < 0 || + virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.devno) < 0) + return -1; + break; + + case VIRSH_ADDRESS_TYPE_USB: + if (virStrToLong_uip(++a, &a, 10, &addr->addr.usb.bus) < 0 || + virStrToLong_uip(++a, &a, 10, &addr->addr.usb.port) < 0) + return -1; + break; + + case VIRSH_ADDRESS_TYPE_LAST: + default: + return -1; + } + + return 0; +} + + +static void +virshAddressFormat(virBufferPtr buf, + struct virshAddress *addr) +{ + switch ((enum virshAddressType) addr->type) { + case VIRSH_ADDRESS_TYPE_PCI: + virBufferAsprintf(buf, + "<address type='pci' domain='0x%04x' bus='0x%02x' slot='0x%02x' function='0x%0x'", + addr->addr.pci.domain, + addr->addr.pci.bus, + addr->addr.pci.slot, + addr->addr.pci.function); + + if (addr->addr.pci.multifunction) + virBufferAddLit(buf, " multifunction='on'"); + + virBufferAddLit(buf, "/>\n"); + break; + + case VIRSH_ADDRESS_TYPE_SATA: + case VIRSH_ADDRESS_TYPE_IDE: + case VIRSH_ADDRESS_TYPE_SCSI: + virBufferAsprintf(buf, + "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", + addr->addr.drive.controller, + addr->addr.drive.bus, + addr->addr.drive.unit); + break; + + case VIRSH_ADDRESS_TYPE_CCW: + virBufferAsprintf(buf, + "<address type='ccw' cssid='0x%02x' ssid='0x%01x' devno='0x%04x'/>\n", + addr->addr.ccw.cssid, + addr->addr.ccw.ssid, + addr->addr.ccw.devno); + break; + + case VIRSH_ADDRESS_TYPE_USB: + virBufferAsprintf(buf, + "<address type='usb' bus='%u' port='%u'/>\n", + addr->addr.usb.bus, + addr->addr.usb.port); + break; + + case VIRSH_ADDRESS_TYPE_LAST: + default: + return; + } +} + + #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \ {.name = "persistent", \ .type = VSH_OT_BOOL, \ @@ -305,177 +477,6 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = NULL} }; -enum virshAddressType { - VIRSH_ADDRESS_TYPE_PCI, - VIRSH_ADDRESS_TYPE_SCSI, - VIRSH_ADDRESS_TYPE_IDE, - VIRSH_ADDRESS_TYPE_CCW, - VIRSH_ADDRESS_TYPE_USB, - VIRSH_ADDRESS_TYPE_SATA, - - VIRSH_ADDRESS_TYPE_LAST -}; - -VIR_ENUM_DECL(virshAddress); -VIR_ENUM_IMPL(virshAddress, - VIRSH_ADDRESS_TYPE_LAST, - "pci", - "scsi", - "ide", - "ccw", - "usb", - "sata"); - -struct virshAddressPCI { - unsigned int domain; - unsigned int bus; - unsigned int slot; - unsigned int function; - bool multifunction; -}; - -struct virshAddressDrive { - unsigned int controller; - unsigned int bus; - unsigned long long unit; -}; - -struct virshAddressCCW { - unsigned int cssid; - unsigned int ssid; - unsigned int devno; -}; - -struct virshAddressUSB { - unsigned int bus; - unsigned int port; -}; - -struct virshAddress { - int type; /* enum virshAddressType */ - union { - struct virshAddressPCI pci; - struct virshAddressDrive drive; - struct virshAddressCCW ccw; - struct virshAddressUSB usb; - } addr; -}; - - -/* pci address pci:0000.00.0x0a.0 (domain:bus:slot:function) - * ide disk address: ide:00.00.0 (controller:bus:unit) - * scsi disk address: scsi:00.00.0 (controller:bus:unit) - * ccw disk address: ccw:0xfe.0.0000 (cssid:ssid:devno) - * usb disk address: usb:00.00 (bus:port) - * sata disk address: sata:00.00.0 (controller:bus:unit) - */ -static int -virshAddressParse(const char *str, - bool multifunction, - struct virshAddress *addr) -{ - g_autofree char *type = g_strdup(str); - char *a = strchr(type, ':'); - - if (!addr) - return -1; - - *a = '\0'; - - addr->type = virshAddressTypeFromString(type); - - switch ((enum virshAddressType) addr->type) { - case VIRSH_ADDRESS_TYPE_PCI: - addr->addr.pci.multifunction = multifunction; - - if (virStrToLong_uip(++a, &a, 16, &addr->addr.pci.domain) < 0 || - virStrToLong_uip(++a, &a, 16, &addr->addr.pci.bus) < 0 || - virStrToLong_uip(++a, &a, 16, &addr->addr.pci.slot) < 0 || - virStrToLong_uip(++a, &a, 16, &addr->addr.pci.function) < 0) - return -1; - break; - - case VIRSH_ADDRESS_TYPE_SATA: - case VIRSH_ADDRESS_TYPE_IDE: - case VIRSH_ADDRESS_TYPE_SCSI: - if (virStrToLong_uip(++a, &a, 10, &addr->addr.drive.controller) < 0 || - virStrToLong_uip(++a, &a, 10, &addr->addr.drive.bus) < 0 || - virStrToLong_ullp(++a, &a, 10, &addr->addr.drive.unit) < 0) - return -1; - break; - - case VIRSH_ADDRESS_TYPE_CCW: - if (virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.cssid) < 0 || - virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.ssid) < 0 || - virStrToLong_uip(++a, &a, 16, &addr->addr.ccw.devno) < 0) - return -1; - break; - - case VIRSH_ADDRESS_TYPE_USB: - if (virStrToLong_uip(++a, &a, 10, &addr->addr.usb.bus) < 0 || - virStrToLong_uip(++a, &a, 10, &addr->addr.usb.port) < 0) - return -1; - break; - - case VIRSH_ADDRESS_TYPE_LAST: - default: - return -1; - } - - return 0; -} - - -static void -virshAddressFormat(virBufferPtr buf, - struct virshAddress *addr) -{ - switch ((enum virshAddressType) addr->type) { - case VIRSH_ADDRESS_TYPE_PCI: - virBufferAsprintf(buf, - "<address type='pci' domain='0x%04x' bus='0x%02x' slot='0x%02x' function='0x%0x'", - addr->addr.pci.domain, - addr->addr.pci.bus, - addr->addr.pci.slot, - addr->addr.pci.function); - - if (addr->addr.pci.multifunction) - virBufferAddLit(buf, " multifunction='on'"); - - virBufferAddLit(buf, "/>\n"); - break; - - case VIRSH_ADDRESS_TYPE_SATA: - case VIRSH_ADDRESS_TYPE_IDE: - case VIRSH_ADDRESS_TYPE_SCSI: - virBufferAsprintf(buf, - "<address type='drive' controller='%u' bus='%u' unit='%llu'/>\n", - addr->addr.drive.controller, - addr->addr.drive.bus, - addr->addr.drive.unit); - break; - - case VIRSH_ADDRESS_TYPE_CCW: - virBufferAsprintf(buf, - "<address type='ccw' cssid='0x%02x' ssid='0x%01x' devno='0x%04x'/>\n", - addr->addr.ccw.cssid, - addr->addr.ccw.ssid, - addr->addr.ccw.devno); - break; - - case VIRSH_ADDRESS_TYPE_USB: - virBufferAsprintf(buf, - "<address type='usb' bus='%u' port='%u'/>\n", - addr->addr.usb.bus, - addr->addr.usb.port); - break; - - case VIRSH_ADDRESS_TYPE_LAST: - default: - return; - } -} - static int cmdAttachDiskFormatAddress(vshControl *ctl, -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 12/16] virsh: cmdAttachDisk: Use enum for disk source type

For extendability and clarity add enum virshAttachDiskSourceType and use it to drive the XML formatting. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 68 +++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f8d97593b8..5c7cb994f1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -519,6 +519,22 @@ cmdAttachDiskFormatAddress(vshControl *ctl, } +enum virshAttachDiskSourceType { + VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE, + VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE, + VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK, + + VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST +}; + +VIR_ENUM_DECL(virshAttachDiskSource); +VIR_ENUM_IMPL(virshAttachDiskSource, + VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST, + "", + "file", + "block"); + + static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { @@ -527,7 +543,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *target = NULL; const char *driver = NULL; const char *subdriver = NULL; - const char *type = NULL; + const char *device = NULL; const char *mode = NULL; const char *iothread = NULL; const char *cache = NULL; @@ -537,10 +553,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *wwn = NULL; const char *targetbus = NULL; const char *alias = NULL; - bool isBlock = false; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; + int type = VIR_STORAGE_TYPE_NONE; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *xml = NULL; struct stat st; @@ -564,7 +580,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0 || - vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 || + vshCommandOptStringReq(ctl, cmd, "type", &device) < 0 || vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0 || vshCommandOptStringReq(ctl, cmd, "iothread", &iothread) < 0 || vshCommandOptStringReq(ctl, cmd, "cache", &cache) < 0 || @@ -577,18 +593,22 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) return false; - if (!stype) { + if (stype && + (type = virshAttachDiskSourceTypeFromString(stype)) < 0) { + vshError(ctl, _("Unknown source type: '%s'"), stype); + return false; + } + + if (type == VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE) { if (STRNEQ_NULLABLE(driver, "file") && STRNEQ_NULLABLE(driver, "tap") && source && stat(source, &st) == 0 && - S_ISBLK(st.st_mode)) - isBlock = true; - } else if (STREQ(stype, "block")) { - isBlock = true; - } else if (STRNEQ(stype, "file")) { - vshError(ctl, _("Unknown source type: '%s'"), stype); - return false; + S_ISBLK(st.st_mode)) { + type = VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK; + } else { + type = VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE; + } } if (mode) { @@ -604,13 +624,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) /* Make XML of disk */ virBufferAddLit(&buf, "<disk"); - if (isBlock) - virBufferAddLit(&buf, " type='block'"); - else - virBufferAddLit(&buf, " type='file'"); + virBufferAsprintf(&buf, " type='%s'", virshAttachDiskSourceTypeToString(type)); - if (type) - virBufferAsprintf(&buf, " device='%s'", type); + if (device) + virBufferAsprintf(&buf, " device='%s'", device); if (vshCommandOptBool(cmd, "rawio")) virBufferAddLit(&buf, " rawio='yes'"); virBufferAddLit(&buf, ">\n"); @@ -633,9 +650,20 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "/>\n"); } - if (source) - virBufferAsprintf(&buf, "<source %s='%s'/>\n", - !isBlock ? "file" : "dev", source); + switch ((enum virshAttachDiskSourceType) type) { + case VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE: + virBufferEscapeString(&buf, "<source file='%s'/>\n", source); + break; + + case VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK: + virBufferEscapeString(&buf, "<source dev='%s'/>\n", source); + break; + + case VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE: + case VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST: + break; + } + virBufferAsprintf(&buf, "<target dev='%s'", target); if (targetbus) virBufferAsprintf(&buf, " bus='%s'", targetbus); -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 13/16] virsh: cmdAttachDisk: Use virXMLFormatElement

Convert the code to the new XML formatting approach for simpler code and future additions. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tools/virsh-domain.c | 61 ++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5c7cb994f1..b0e1c6e6f7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -558,6 +558,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *stype = NULL; int type = VIR_STORAGE_TYPE_NONE; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) diskAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) diskChildBuf = VIR_BUFFER_INIT_CHILD(&buf); + g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) sourceAttrBuf = VIR_BUFFER_INITIALIZER; g_autofree char *xml = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); @@ -622,71 +626,56 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (wwn && !virValidateWWN(wwn)) return false; - /* Make XML of disk */ - virBufferAddLit(&buf, "<disk"); - virBufferAsprintf(&buf, " type='%s'", virshAttachDiskSourceTypeToString(type)); - - if (device) - virBufferAsprintf(&buf, " device='%s'", device); + virBufferAsprintf(&diskAttrBuf, " type='%s'", virshAttachDiskSourceTypeToString(type)); + virBufferEscapeString(&diskAttrBuf, " device='%s'", device); if (vshCommandOptBool(cmd, "rawio")) - virBufferAddLit(&buf, " rawio='yes'"); - virBufferAddLit(&buf, ">\n"); - virBufferAdjustIndent(&buf, 2); - - if (driver || subdriver || iothread || cache || io) { - virBufferAddLit(&buf, "<driver"); + virBufferAddLit(&diskAttrBuf, " rawio='yes'"); - if (driver) - virBufferAsprintf(&buf, " name='%s'", driver); - if (subdriver) - virBufferAsprintf(&buf, " type='%s'", subdriver); - if (iothread) - virBufferAsprintf(&buf, " iothread='%s'", iothread); - if (cache) - virBufferAsprintf(&buf, " cache='%s'", cache); - if (io) - virBufferAsprintf(&buf, " io='%s'", io); + virBufferEscapeString(&driverAttrBuf, " name='%s'", driver); + virBufferEscapeString(&driverAttrBuf, " type='%s'", subdriver); + virBufferEscapeString(&driverAttrBuf, " iothread='%s'", iothread); + virBufferEscapeString(&driverAttrBuf, " cache='%s'", cache); + virBufferEscapeString(&driverAttrBuf, " io='%s'", io); - virBufferAddLit(&buf, "/>\n"); - } + virXMLFormatElement(&diskChildBuf, "driver", &driverAttrBuf, NULL); switch ((enum virshAttachDiskSourceType) type) { case VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE: - virBufferEscapeString(&buf, "<source file='%s'/>\n", source); + virBufferEscapeString(&sourceAttrBuf, " file='%s'", source); break; case VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK: - virBufferEscapeString(&buf, "<source dev='%s'/>\n", source); + virBufferEscapeString(&sourceAttrBuf, " dev='%s'", source); break; case VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE: case VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST: break; } + virXMLFormatElement(&diskChildBuf, "source", &sourceAttrBuf, NULL); - virBufferAsprintf(&buf, "<target dev='%s'", target); + virBufferAsprintf(&diskChildBuf, "<target dev='%s'", target); if (targetbus) - virBufferAsprintf(&buf, " bus='%s'", targetbus); - virBufferAddLit(&buf, "/>\n"); + virBufferAsprintf(&diskChildBuf, " bus='%s'", targetbus); + virBufferAddLit(&diskChildBuf, "/>\n"); if (mode) - virBufferAsprintf(&buf, "<%s/>\n", mode); + virBufferAsprintf(&diskChildBuf, "<%s/>\n", mode); if (serial) - virBufferAsprintf(&buf, "<serial>%s</serial>\n", serial); + virBufferAsprintf(&diskChildBuf, "<serial>%s</serial>\n", serial); if (alias) - virBufferAsprintf(&buf, "<alias name='%s'/>\n", alias); + virBufferAsprintf(&diskChildBuf, "<alias name='%s'/>\n", alias); if (wwn) - virBufferAsprintf(&buf, "<wwn>%s</wwn>\n", wwn); + virBufferAsprintf(&diskChildBuf, "<wwn>%s</wwn>\n", wwn); if (straddr && - cmdAttachDiskFormatAddress(ctl, &buf, straddr, target, multifunction) < 0) + cmdAttachDiskFormatAddress(ctl, &diskChildBuf, straddr, target, multifunction) < 0) return false; - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</disk>\n"); + virXMLFormatElement(&buf, "disk", &diskAttrBuf, &diskChildBuf); xml = virBufferContentAndReset(&buf); -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 14/16] virsh: Added attach-disk support for network disk

From: Ryan Gahagan <rgahagan(a)cs.utexas.edu> Related issue: https://gitlab.com/libvirt/libvirt/-/issues/16 Added in support for the following parameters in attach-disk: --source-protocol --source-host-name --source-host-socket --source-host-transport Added documentation to virsh.rst specifying usage. Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- docs/manpages/virsh.rst | 31 ++++++++++--- tests/virsh-output-commands | 14 ++++++ tests/virsh-output.out | 88 +++++++++++++++++++++++++++++++++++++ tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++----- 4 files changed, 200 insertions(+), 17 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 1ae6d1a0d4..36c868a3e6 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4538,14 +4538,18 @@ attach-disk [--current]] | [--persistent]] [--targetbus bus] [--driver driver] [--subdriver subdriver] [--iothread iothread] [--cache cache] [--io io] [--type type] [--alias alias] - [--mode mode] [--sourcetype sourcetype] [--serial serial] - [--wwn wwn] [--rawio] [--address address] [--multifunction] - [--print-xml] + [--mode mode] [--sourcetype sourcetype] + [--source-protocol protocol] [--source-host-name hostname:port] + [--source-host-transport transport] [--source-host-socket socket] + [--serial serial] [--wwn wwn] [--rawio] [--address address] + [--multifunction] [--print-xml] Attach a new disk device to the domain. -*source* is path for the files and devices. *target* controls the bus or -device under which the disk is exposed to the guest OS. It indicates the -"logical" device name; the optional *targetbus* attribute specifies the type +*source* is path for the files and devices unless *--source-protocol* +is specified, in which case *source* is the name of a network disk. +*target* controls the bus or device under which the disk is exposed +to the guest OS. It indicates the "logical" device name; +the optional *targetbus* attribute specifies the type of disk device to emulate; possible values are driver specific, with typical values being *ide*, *scsi*, *virtio*, *xen*, *usb*, *sata*, or *sd*, if omitted, the bus type is inferred from the style of the device name (e.g. a @@ -4563,7 +4567,7 @@ within the existing virtual cdrom or floppy device; consider using ``update-device`` for this usage instead. *alias* can set user supplied alias. *mode* can specify the two specific mode *readonly* or *shareable*. -*sourcetype* can indicate the type of source (block|file) +*sourcetype* can indicate the type of source (block|file|network) *cache* can be one of "default", "none", "writethrough", "writeback", "directsync" or "unsafe". *io* controls specific policies on I/O; QEMU guests support "threads", @@ -4579,6 +4583,19 @@ ccw:cssid.ssid.devno. Virtio-ccw devices must have their cssid set to 0xfe. *multifunction* indicates specified pci address is a multifunction pci device address. +There is also support for using a network disk. As specified, the user can +provide a *--source-protocol* in which case the *source* parameter will +be interpreted as the source name. *--source-protocol* must be provided +if the user intends to provide a network disk or host information. +Host information can be provided using the tags +*--source-host-name*, *--source-host-transport*, and *--source-host-socket*, +which respectively denote the name of the host, the host's transport method, +and the socket that the host uses. *--source-host-socket* and +*--source-host-name* cannot both be provided, and the user must provide a +*--source-host-transport* if they want to provide a *--source-host-socket*. +The *--source-host-name* parameter supports host:port syntax +if the user wants to provide a port as well. + If *--print-xml* is specified, then the XML of the disk that would be attached is printed instead. diff --git a/tests/virsh-output-commands b/tests/virsh-output-commands index 43f87bee0f..d8e73fc1ac 100755 --- a/tests/virsh-output-commands +++ b/tests/virsh-output-commands @@ -78,3 +78,17 @@ attach_disk --target sda --sourcetype file --type disk --address ccw:12.34.56 attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 attach_disk --target vda --sourcetype file --type disk --address test:12:34:56 attach_disk --target vda --sourcetype file --type disk --address test:12.34.56 + +./virsh attach-disk --print-xml --domain testdom $@ --source "" --source-protocol AAA +./virsh attach-disk --print-xml --domain testdom $@ --source "" --source-protocol AAA +attach_disk --target hda --source-protocol AAA --sourcetype file +attach_disk --target hda --sourcetype network +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname: +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name :port +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name : +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port --source-host-transport trnsp +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp --source-host-socket /nonexistent/socket +attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-socket /nonexistent/socket diff --git a/tests/virsh-output.out b/tests/virsh-output.out index 6af7600a9c..d8ef641c2a 100644 --- a/tests/virsh-output.out +++ b/tests/virsh-output.out @@ -405,4 +405,92 @@ error: Invalid address. + ./virsh attach-disk --print-xml --domain testdom --target vda --sourcetype file --type disk --address test:12.34.56 --source /nonexistent/file error: Invalid address. ++ ./virsh attach-disk --print-xml --domain testdom --source '' --source-protocol AAA +error: command 'attach-disk' requires <target> option ++ ./virsh attach-disk --print-xml --domain testdom --source '' --source-protocol AAA +error: command 'attach-disk' requires <target> option ++ attach_disk --target hda --source-protocol AAA --sourcetype file ++ ./virsh attach-disk --print-xml --domain testdom --target hda --source-protocol AAA --sourcetype file --source /nonexistent/file +error: --source-protocol option requires --sourcetype network + ++ attach_disk --target hda --sourcetype network ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source /nonexistent/file +error: --source-protocol option requires --sourcetype network + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name hostname --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host name='hostname'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host name='hostname' port='port'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname: ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name hostname: --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host name='hostname' port=''/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name :port ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name :port --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host name='' port='port'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name : ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name : --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host name='' port=''/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port --source-host-transport trnsp ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-name hostname:port --source-host-transport trnsp --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host transport='trnsp' name='hostname' port='port'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host transport='trnsp'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp --source-host-socket /nonexistent/socket ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-transport trnsp --source-host-socket /nonexistent/socket --source /nonexistent/file +<disk type='network'> + <source protocol='TEST' name='/nonexistent/file'> + <host transport='trnsp' socket='/nonexistent/socket'/> + </source> + <target dev='hda'/> +</disk> + ++ attach_disk --target hda --sourcetype network --source-protocol TEST --source-host-socket /nonexistent/socket ++ ./virsh attach-disk --print-xml --domain testdom --target hda --sourcetype network --source-protocol TEST --source-host-socket /nonexistent/socket --source /nonexistent/file +error: Option --source-host-transport is required by option --source-host-socket + end diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b0e1c6e6f7..7203403b31 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -394,7 +394,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = "source", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK, - .help = N_("source of disk device") + .help = N_("source of disk device or name of network disk") }, {.name = "target", .type = VSH_OT_DATA, @@ -440,7 +440,7 @@ static const vshCmdOptDef opts_attach_disk[] = { }, {.name = "sourcetype", .type = VSH_OT_STRING, - .help = N_("type of source (block|file)") + .help = N_("type of source (block|file|network)") }, {.name = "serial", .type = VSH_OT_STRING, @@ -470,6 +470,22 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_BOOL, .help = N_("print XML document rather than attach the disk") }, + {.name = "source-protocol", + .type = VSH_OT_STRING, + .help = N_("protocol used by disk device source") + }, + {.name = "source-host-name", + .type = VSH_OT_STRING, + .help = N_("host name for source of disk device") + }, + {.name = "source-host-transport", + .type = VSH_OT_STRING, + .help = N_("host transport for source of disk device") + }, + {.name = "source-host-socket", + .type = VSH_OT_STRING, + .help = N_("host socket for source of disk device") + }, VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, VIRSH_COMMON_OPT_DOMAIN_CONFIG, VIRSH_COMMON_OPT_DOMAIN_LIVE, @@ -523,6 +539,7 @@ enum virshAttachDiskSourceType { VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE, VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE, VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK, + VIRSH_ATTACH_DISK_SOURCE_TYPE_NETWORK, VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST }; @@ -532,7 +549,8 @@ VIR_ENUM_IMPL(virshAttachDiskSource, VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST, "", "file", - "block"); + "block", + "network"); static bool @@ -553,6 +571,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *wwn = NULL; const char *targetbus = NULL; const char *alias = NULL; + const char *source_protocol = NULL; + const char *host_name = NULL; + const char *host_transport = NULL; + const char *host_socket = NULL; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; @@ -562,6 +584,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) g_auto(virBuffer) diskChildBuf = VIR_BUFFER_INIT_CHILD(&buf); g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) sourceAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) sourceChildBuf = VIR_BUFFER_INIT_CHILD(&diskChildBuf); + g_auto(virBuffer) hostAttrBuf = VIR_BUFFER_INITIALIZER; g_autofree char *xml = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); @@ -575,6 +599,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + VSH_REQUIRE_OPTION("source-host-name", "source-protocol"); + VSH_REQUIRE_OPTION("source-host-transport", "source-protocol"); + VSH_REQUIRE_OPTION("source-host-socket", "source-protocol"); + VSH_REQUIRE_OPTION("source-host-socket", "source-host-transport"); + + VSH_EXCLUSIVE_OPTIONS("source-host-name", "source-host-socket"); + if (config || persistent) flags |= VIR_DOMAIN_AFFECT_CONFIG; if (live) @@ -594,7 +625,11 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) + vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-protocol", &source_protocol) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) return false; if (stype && @@ -604,17 +639,24 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } if (type == VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE) { - if (STRNEQ_NULLABLE(driver, "file") && - STRNEQ_NULLABLE(driver, "tap") && - source && - stat(source, &st) == 0 && - S_ISBLK(st.st_mode)) { + if (source_protocol) { + type = VIRSH_ATTACH_DISK_SOURCE_TYPE_NETWORK; + } else if (STRNEQ_NULLABLE(driver, "file") && + STRNEQ_NULLABLE(driver, "tap") && + source && + stat(source, &st) == 0 && + S_ISBLK(st.st_mode)) { type = VIRSH_ATTACH_DISK_SOURCE_TYPE_BLOCK; } else { type = VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE; } } + if ((type == VIRSH_ATTACH_DISK_SOURCE_TYPE_NETWORK) != !!source_protocol) { + vshError(ctl, _("--source-protocol option requires --sourcetype network")); + return false; + } + if (mode) { if (STRNEQ(mode, "readonly") && STRNEQ(mode, "shareable")) { vshError(ctl, _("No support for %s in command 'attach-disk'"), @@ -648,11 +690,33 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferEscapeString(&sourceAttrBuf, " dev='%s'", source); break; + case VIRSH_ATTACH_DISK_SOURCE_TYPE_NETWORK: + virBufferEscapeString(&sourceAttrBuf, " protocol='%s'", source_protocol); + virBufferEscapeString(&sourceAttrBuf, " name='%s'", source); + + virBufferEscapeString(&hostAttrBuf, " transport='%s'", host_transport); + virBufferEscapeString(&hostAttrBuf, " socket='%s'", host_socket); + + if (host_name) { + g_autofree char *host_name_copy = g_strdup(host_name); + char *host_port = strchr(host_name_copy, ':'); + + if (host_port) { + *host_port = '\0'; + host_port++; + } + + virBufferEscapeString(&hostAttrBuf, " name='%s'", host_name_copy); + virBufferEscapeString(&hostAttrBuf, " port='%s'", host_port); + } + virXMLFormatElement(&sourceChildBuf, "host", &hostAttrBuf, NULL); + break; + case VIRSH_ATTACH_DISK_SOURCE_TYPE_NONE: case VIRSH_ATTACH_DISK_SOURCE_TYPE_LAST: break; } - virXMLFormatElement(&diskChildBuf, "source", &sourceAttrBuf, NULL); + virXMLFormatElement(&diskChildBuf, "source", &sourceAttrBuf, &sourceChildBuf); virBufferAsprintf(&diskChildBuf, "<target dev='%s'", target); if (targetbus) -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 15/16] tests: virsh-output: Mark as expensive

The test takes more than a second on a beefy machine. While it's more useful than some expensive tests it's not worth running all the time. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/virsh-output | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/virsh-output b/tests/virsh-output index 44833c7048..aa2d386c39 100755 --- a/tests/virsh-output +++ b/tests/virsh-output @@ -2,6 +2,8 @@ . "$(dirname $0)/test-lib.sh" +test_expensive + if [ ! -x /bin/bash ]; then echo skipping test - no bash exit 77 -- 2.28.0

Peter Krempa

10:26 a.m.

New subject: [PATCH 16/16] tests: virsh-checkpoint/snapshot: Mark as expensive

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/virsh-checkpoint | 2 ++ tests/virsh-snapshot | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/virsh-checkpoint b/tests/virsh-checkpoint index 92962d2205..2492d29f53 100755 --- a/tests/virsh-checkpoint +++ b/tests/virsh-checkpoint @@ -19,6 +19,8 @@ . "$(dirname $0)/test-lib.sh" +test_expensive + if test "$VERBOSE" = yes; then set -x $abs_top_builddir/tools/virsh --version diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot index 473e3b3f2d..289de5b2db 100755 --- a/tests/virsh-snapshot +++ b/tests/virsh-snapshot @@ -19,6 +19,8 @@ . "$(dirname $0)/test-lib.sh" +test_expensive + if test "$VERBOSE" = yes; then set -x $abs_top_builddir/tools/virsh --version -- 2.28.0

Daniel Henrique Barboza

Friday, 20 November Fri, 20 Nov

5:57 a.m.

On 11/19/20 1:26 PM, Peter Krempa wrote:

...

This version refactors cmdAttachDisk first so that we can use a much simpler logic to format the XML. With the simpler logic and tests the code now doesn't misformat the XML in certain situations as it was with the previous posting: virsh attach-disk --print-xml --domain ble --source "asdfsource" --target "tgt" > <source dev='asdfsource'/> <target dev='tgt'/> </disk> I've rather rewrote how we do things than trying to investigate why the original version [1] failed. The new version also adds tests so it's simpler to see that things actually work. [1] https://www.redhat.com/archives/libvir-list/2020-November/msg01071.html

Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>

...

Peter Krempa (15): tests: virsh: Add simple testsuite for using --print-xml of various commands virsh: cmdAttachDisk: Declare one variable per line virsh: cmdAttachDisk: Use automatic memory clearing for 'xml' and 'dom' virsh: cmdAttachDisk: Refactor control flow virsh: cmdAttachDisk: Don't assume type='block' if file can't be examined virsh: cmdAttachDisk: Split out formatting of disk address virsh: Extract address formatting from cmdAttachDiskFormatAddress virsh: Unify code for <address type='drive' virsh: Refactor str2DiskAddress virsh: Rename 'struct DiskAddress' and friends virsh: virshAddress*: Move out of cmdAttachDisk block virsh: cmdAttachDisk: Use enum for disk source type virsh: cmdAttachDisk: Use virXMLFormatElement tests: virsh-output: Mark as expensive tests: virsh-checkpoint/snapshot: Mark as expensive Ryan Gahagan (1): virsh: Added attach-disk support for network disk docs/manpages/virsh.rst | 31 +- tests/meson.build | 1 + tests/virsh-checkpoint | 2 + tests/virsh-output | 29 ++ tests/virsh-output-commands | 94 +++++ tests/virsh-output.out | 496 ++++++++++++++++++++++++ tests/virsh-snapshot | 2 + tools/virsh-domain.c | 748 ++++++++++++++++++------------------ 8 files changed, 1017 insertions(+), 386 deletions(-) create mode 100755 tests/virsh-output create mode 100755 tests/virsh-output-commands create mode 100644 tests/virsh-output.out

1450

days inactive

1451

days old

devel@lists.libvirt.org

Manage subscription

18 comments

2 participants

tags (0)

participants (2)

Daniel Henrique Barboza
Peter Krempa

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[PATCH 00/16] virsh: Add support for network disks to --attach-disk