[libvirt] [PATCH 00/17] chardev parsing cleanup and improvements
Pavel Hrdina (17): tests: introduce genericxml test for TCP chardev tests: introduce genericxml test for UDP chardev tests: introduce genericxml test for UNIX chardev conf: switch from while to for loop for chardev parsing conf: error out for multiple source elements while parsing chardev conf: error out for multiple log elements while parsing chardev conf: error out for multiple protocol elements while parsing chardev conf: move chardev protocol parsing to separate function conf: move chardev log parsing to separate function conf: move mode parsing of chardev source to separate function conf: move TCP chardev source parsing to separate function conf: move UDP chardev source parsing to separate function conf: move UNIX chardev source parsing to separate function conf: assign parsed strings directly into chardev source definition conf: move FILE chardev source parsing to separate function conf: separate PTY chardev source parsing conf: move chardev validation into virDomainDeviceDefValidateInternal src/conf/domain_conf.c | 700 ++++++++++++--------- .../generic-chardev-tcp-missing-host.xml | 25 + .../generic-chardev-tcp-missing-service.xml | 25 + .../generic-chardev-tcp-multiple-source.xml | 26 + tests/genericxml2xmlindata/generic-chardev-tcp.xml | 32 + ...generic-chardev-udp-missing-connect-service.xml | 24 + .../generic-chardev-udp-multiple-source.xml | 26 + tests/genericxml2xmlindata/generic-chardev-udp.xml | 47 ++ .../generic-chardev-unix-redirdev-missing-path.xml | 24 + .../generic-chardev-unix-rng-missing-path.xml | 25 + ...generic-chardev-unix-smartcard-missing-path.xml | 23 + .../genericxml2xmlindata/generic-chardev-unix.xml | 43 ++ .../genericxml2xmloutdata/generic-chardev-tcp.xml | 35 ++ .../genericxml2xmloutdata/generic-chardev-udp.xml | 47 ++ .../genericxml2xmloutdata/generic-chardev-unix.xml | 44 ++ tests/genericxml2xmltest.c | 20 + 16 files changed, 875 insertions(+), 291 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-tcp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-udp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-unix.xml -- 2.13.5
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../generic-chardev-tcp-missing-host.xml | 25 ++++++++++++++++ .../generic-chardev-tcp-missing-service.xml | 25 ++++++++++++++++ tests/genericxml2xmlindata/generic-chardev-tcp.xml | 32 ++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-tcp.xml | 35 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 6 ++++ 5 files changed, 123 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-tcp.xml diff --git a/tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml b/tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml new file mode 100644 index 0000000000..523c34f29b --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='tcp'> + <source mode='bind' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='test2'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml b/tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml new file mode 100644 index 0000000000..880f07436a --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='tcp'> + <source mode='bind' host='localhost'/> + <protocol type='raw'/> + <target type='virtio' name='test1'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-tcp.xml b/tests/genericxml2xmlindata/generic-chardev-tcp.xml new file mode 100644 index 0000000000..e59c572a12 --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-tcp.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='tcp'> + <source host='localhost' service='5678'/> + <target type='virtio' name='test1'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='localhost' service='5678'/> + <target type='virtio' name='test2'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='localhost' service='5678'/> + <target type='virtio' name='test3'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-chardev-tcp.xml b/tests/genericxml2xmloutdata/generic-chardev-tcp.xml new file mode 100644 index 0000000000..ef217c0abb --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-chardev-tcp.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='tcp'> + <source mode='connect' host='localhost' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='test1'/> + </channel> + <channel type='tcp'> + <source mode='connect' host='localhost' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='test2'/> + </channel> + <channel type='tcp'> + <source mode='bind' host='localhost' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='test3'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 5626eedf43..f5ad001968 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -105,6 +105,12 @@ mymain(void) DO_TEST("cpu-cache-passthrough"); DO_TEST("cpu-cache-disable"); + DO_TEST_DIFFERENT("chardev-tcp"); + DO_TEST_FULL("chardev-tcp-missing-host", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("chardev-tcp-missing-service", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:01AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../generic-chardev-tcp-missing-host.xml | 25 ++++++++++++++++ .../generic-chardev-tcp-missing-service.xml | 25 ++++++++++++++++ tests/genericxml2xmlindata/generic-chardev-tcp.xml | 32 ++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-tcp.xml | 35 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 6 ++++ 5 files changed, 123 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-host.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-missing-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-tcp.xml
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- ...generic-chardev-udp-missing-connect-service.xml | 24 +++++++++++ tests/genericxml2xmlindata/generic-chardev-udp.xml | 47 ++++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-udp.xml | 47 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 3 ++ 4 files changed, 121 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-udp.xml diff --git a/tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml b/tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml new file mode 100644 index 0000000000..c7ed52b457 --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='udp'> + <source mode='connect' host='localhost'/> + <target type='virtio' name='test1'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-udp.xml b/tests/genericxml2xmlindata/generic-chardev-udp.xml new file mode 100644 index 0000000000..34441ae68e --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-udp.xml @@ -0,0 +1,47 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='udp'> + <source service='5678'/> + <target type='virtio' name='test1'/> + </channel> + <channel type='udp'> + <source mode='connect' service='5678'/> + <target type='virtio' name='test2'/> + </channel> + <channel type='udp'> + <source mode='connect' service='5678'/> + <source mode='bind' service='4567'/> + <target type='virtio' name='test3'/> + </channel> + <channel type='udp'> + <source mode='connect' service='5678'/> + <source mode='bind' host='localhost'/> + <target type='virtio' name='test4'/> + </channel> + <channel type='udp'> + <source mode='connect' host='localhost' service='5678'/> + <target type='virtio' name='test5'/> + </channel> + <channel type='udp'> + <source mode='connect' host='localhost' service='5678'/> + <source mode='bind' host='localhost' service='4567'/> + <target type='virtio' name='test6'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-chardev-udp.xml b/tests/genericxml2xmloutdata/generic-chardev-udp.xml new file mode 100644 index 0000000000..c4a719f2f6 --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-chardev-udp.xml @@ -0,0 +1,47 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='udp'> + <source mode='connect' service='5678'/> + <target type='virtio' name='test1'/> + </channel> + <channel type='udp'> + <source mode='connect' service='5678'/> + <target type='virtio' name='test2'/> + </channel> + <channel type='udp'> + <source mode='bind' service='4567'/> + <source mode='connect' service='5678'/> + <target type='virtio' name='test3'/> + </channel> + <channel type='udp'> + <source mode='bind'/> + <source mode='connect' service='5678'/> + <target type='virtio' name='test4'/> + </channel> + <channel type='udp'> + <source mode='connect' host='localhost' service='5678'/> + <target type='virtio' name='test5'/> + </channel> + <channel type='udp'> + <source mode='bind' host='localhost' service='4567'/> + <source mode='connect' host='localhost' service='5678'/> + <target type='virtio' name='test6'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index f5ad001968..c10c3f5c04 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -110,6 +110,9 @@ mymain(void) TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_FULL("chardev-tcp-missing-service", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_DIFFERENT("chardev-udp"); + DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:02AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- ...generic-chardev-udp-missing-connect-service.xml | 24 +++++++++++ tests/genericxml2xmlindata/generic-chardev-udp.xml | 47 ++++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-udp.xml | 47 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 3 ++ 4 files changed, 121 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-missing-connect-service.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-udp.xml
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../generic-chardev-unix-redirdev-missing-path.xml | 24 ++++++++++++ .../generic-chardev-unix-rng-missing-path.xml | 25 ++++++++++++ ...generic-chardev-unix-smartcard-missing-path.xml | 23 +++++++++++ .../genericxml2xmlindata/generic-chardev-unix.xml | 43 +++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-unix.xml | 44 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 7 ++++ 6 files changed, 166 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-unix.xml diff --git a/tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml b/tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml new file mode 100644 index 0000000000..f0d6f3d60a --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <controller type='usb'/> + <redirdev bus='usb' type='unix'> + <source mode='connect'/> + </redirdev> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml b/tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml new file mode 100644 index 0000000000..fb45c044f8 --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <memballoon model='none'/> + <rng model='virtio'> + <backend model='egd' type='unix'> + <source mode='connect'/> + </backend> + </rng> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml b/tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml new file mode 100644 index 0000000000..451c8aa660 --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <smartcard mode='passthrough' type='unix'> + <source mode='connect'/> + </smartcard> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-unix.xml b/tests/genericxml2xmlindata/generic-chardev-unix.xml new file mode 100644 index 0000000000..bc715a3ae8 --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-unix.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <controller type='usb'/> + <smartcard mode='passthrough' type='unix'> + <source path='/unix/socket/path'/> + </smartcard> + <channel type='unix'> + <target type='virtio' name='test1'/> + </channel> + <channel type='unix'> + <source mode='connect'/> + <target type='virtio' name='test2'/> + </channel> + <channel type='unix'> + <source mode='connect' path='/unix/socket/path'/> + <target type='virtio' name='test3'/> + </channel> + <redirdev bus='usb' type='unix'> + <source path='/unix/socket/path'/> + </redirdev> + <memballoon model='none'/> + <rng model='virtio'> + <backend model='egd' type='unix'> + <source path='/unix/socket/path'/> + </backend> + </rng> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-chardev-unix.xml b/tests/genericxml2xmloutdata/generic-chardev-unix.xml new file mode 100644 index 0000000000..c797cbff8a --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-chardev-unix.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <controller type='usb' index='0'/> + <controller type='ccid' index='0'/> + <smartcard mode='passthrough' type='unix'> + <source mode='connect' path='/unix/socket/path'/> + <address type='ccid' controller='0' slot='0'/> + </smartcard> + <channel type='unix'> + <target type='virtio' name='test1'/> + </channel> + <channel type='unix'> + <target type='virtio' name='test2'/> + </channel> + <channel type='unix'> + <source mode='connect' path='/unix/socket/path'/> + <target type='virtio' name='test3'/> + </channel> + <redirdev bus='usb' type='unix'> + <source mode='connect' path='/unix/socket/path'/> + </redirdev> + <memballoon model='none'/> + <rng model='virtio'> + <backend model='egd' type='unix'> + <source mode='connect' path='/unix/socket/path'/> + </backend> + </rng> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index c10c3f5c04..7dc137ed16 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -113,6 +113,13 @@ mymain(void) DO_TEST_DIFFERENT("chardev-udp"); DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_DIFFERENT("chardev-unix"); + DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("chardev-unix-rng-missing-path", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:03AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../generic-chardev-unix-redirdev-missing-path.xml | 24 ++++++++++++ .../generic-chardev-unix-rng-missing-path.xml | 25 ++++++++++++ ...generic-chardev-unix-smartcard-missing-path.xml | 23 +++++++++++ .../genericxml2xmlindata/generic-chardev-unix.xml | 43 +++++++++++++++++++++ .../genericxml2xmloutdata/generic-chardev-unix.xml | 44 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 7 ++++ 6 files changed, 166 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-redirdev-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-rng-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix-smartcard-missing-path.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-unix.xml create mode 100644 tests/genericxml2xmloutdata/generic-chardev-unix.xml
ACK Jan
This removes one level of indentation. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 168 ++++++++++++++++++++++++------------------------- 1 file changed, 84 insertions(+), 84 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3bef5bed3b..ba0241cb21 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10918,100 +10918,100 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *haveTLS = NULL; char *tlsFromConfig = NULL; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (virXMLNodeNameEqual(cur, "source")) { - if (!mode) - mode = virXMLPropString(cur, "mode"); - if (!haveTLS) - haveTLS = virXMLPropString(cur, "tls"); - if (!tlsFromConfig) - tlsFromConfig = virXMLPropString(cur, "tlsFromConfig"); + for (; cur; cur = cur->next) { + if (cur->type != XML_ELEMENT_NODE) + continue; - switch ((virDomainChrType) def->type) { - case VIR_DOMAIN_CHR_TYPE_FILE: - case VIR_DOMAIN_CHR_TYPE_PTY: - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_PIPE: - case VIR_DOMAIN_CHR_TYPE_UNIX: - if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) - append = virXMLPropString(cur, "append"); - /* PTY path is only parsed from live xml. */ - if (!path && - (def->type != VIR_DOMAIN_CHR_TYPE_PTY || - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))) - path = virXMLPropString(cur, "path"); + if (virXMLNodeNameEqual(cur, "source")) { + if (!mode) + mode = virXMLPropString(cur, "mode"); + if (!haveTLS) + haveTLS = virXMLPropString(cur, "tls"); + if (!tlsFromConfig) + tlsFromConfig = virXMLPropString(cur, "tlsFromConfig"); - break; + switch ((virDomainChrType) def->type) { + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) + append = virXMLPropString(cur, "append"); + /* PTY path is only parsed from live xml. */ + if (!path && + (def->type != VIR_DOMAIN_CHR_TYPE_PTY || + !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))) + path = virXMLPropString(cur, "path"); - case VIR_DOMAIN_CHR_TYPE_UDP: - case VIR_DOMAIN_CHR_TYPE_TCP: - if (!mode || STREQ(mode, "connect")) { - if (!connectHost) - connectHost = virXMLPropString(cur, "host"); - if (!connectService) - connectService = virXMLPropString(cur, "service"); - } else if (STREQ(mode, "bind")) { - if (!bindHost) - bindHost = virXMLPropString(cur, "host"); - if (!bindService) - bindService = virXMLPropString(cur, "service"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown source mode '%s'"), mode); - goto error; - } + break; - if (def->type == VIR_DOMAIN_CHR_TYPE_UDP) - VIR_FREE(mode); - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - if (!channel) - channel = virXMLPropString(cur, "channel"); - break; - - case VIR_DOMAIN_CHR_TYPE_NMDM: - if (!master) - master = virXMLPropString(cur, "master"); - if (!slave) - slave = virXMLPropString(cur, "slave"); - break; - - case VIR_DOMAIN_CHR_TYPE_LAST: - case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_VC: - case VIR_DOMAIN_CHR_TYPE_STDIO: - case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - break; + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + if (!mode || STREQ(mode, "connect")) { + if (!connectHost) + connectHost = virXMLPropString(cur, "host"); + if (!connectService) + connectService = virXMLPropString(cur, "service"); + } else if (STREQ(mode, "bind")) { + if (!bindHost) + bindHost = virXMLPropString(cur, "host"); + if (!bindService) + bindService = virXMLPropString(cur, "service"); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown source mode '%s'"), mode); + goto error; } - /* Check for an optional seclabel override in <source/>. */ - if (chr_def) { - xmlNodePtr saved_node = ctxt->node; - ctxt->node = cur; - if (virSecurityDeviceLabelDefParseXML(&def->seclabels, - &def->nseclabels, - vmSeclabels, - nvmSeclabels, - ctxt, - flags) < 0) { - ctxt->node = saved_node; - goto error; - } + if (def->type == VIR_DOMAIN_CHR_TYPE_UDP) + VIR_FREE(mode); + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!channel) + channel = virXMLPropString(cur, "channel"); + break; + + case VIR_DOMAIN_CHR_TYPE_NMDM: + if (!master) + master = virXMLPropString(cur, "master"); + if (!slave) + slave = virXMLPropString(cur, "slave"); + break; + + case VIR_DOMAIN_CHR_TYPE_LAST: + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + break; + } + + /* Check for an optional seclabel override in <source/>. */ + if (chr_def) { + xmlNodePtr saved_node = ctxt->node; + ctxt->node = cur; + if (virSecurityDeviceLabelDefParseXML(&def->seclabels, + &def->nseclabels, + vmSeclabels, + nvmSeclabels, + ctxt, + flags) < 0) { ctxt->node = saved_node; + goto error; } - } else if (virXMLNodeNameEqual(cur, "log")) { - if (!logfile) - logfile = virXMLPropString(cur, "file"); - if (!logappend) - logappend = virXMLPropString(cur, "append"); - } else if (virXMLNodeNameEqual(cur, "protocol")) { - if (!protocol) - protocol = virXMLPropString(cur, "type"); + ctxt->node = saved_node; } + } else if (virXMLNodeNameEqual(cur, "log")) { + if (!logfile) + logfile = virXMLPropString(cur, "file"); + if (!logappend) + logappend = virXMLPropString(cur, "append"); + } else if (virXMLNodeNameEqual(cur, "protocol")) { + if (!protocol) + protocol = virXMLPropString(cur, "type"); } - cur = cur->next; } switch ((virDomainChrType) def->type) { -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:04AM +0200, Pavel Hrdina wrote:
This removes one level of indentation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Forget to include notes: hint: review with -b
On Mon, Aug 21, 2017 at 10:07:04AM +0200, Pavel Hrdina wrote:
This removes one level of indentation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 168 ++++++++++++++++++++++++------------------------- 1 file changed, 84 insertions(+), 84 deletions(-)
ACK Jan
Currently we accept and correctly parse this chardev XML: ... <channel type='tcp'> <source mode='connect'/> <source mode='bind' host='localhost'/> <source service='4567'/> <target type='virtio' name='test'/> </channel> ... The parsed formatted XML is: ... <channel type='tcp'> <source mode='connect' host='localhost' service='4567'/> <target type='virtio' name='test'/> </channel> ... That behavior is super wrong and should not be allowed. If you notice the current parse takes the first found attribute and uses that value, so for example from the "<source mode='bind' host='localhost'/>" only the "host" attribute is used. It works the same way for all possible attributes that we are able to parse for source element. This patch enforces providing only one source element for all character devices, only for UDP type we allow to provide two source elements since you can specify both modes. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++ .../generic-chardev-tcp-multiple-source.xml | 26 ++++++++++++++++++++++ .../generic-chardev-udp-multiple-source.xml | 26 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 4 ++++ 4 files changed, 73 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ba0241cb21..651a049cf1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10917,12 +10917,29 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *append = NULL; char *haveTLS = NULL; char *tlsFromConfig = NULL; + int sourceParsed = 0; for (; cur; cur = cur->next) { if (cur->type != XML_ELEMENT_NODE) continue; if (virXMLNodeNameEqual(cur, "source")) { + /* Parse only the first source element since only one is used + * for chardev devices, the only exception is UDP type, where + * user can specify two source elements. */ + if (sourceParsed >= 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one source element is allowed for " + "character device")); + goto error; + } else if (sourceParsed >= 2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only two source elements are allowed for " + "character device")); + goto error; + } + sourceParsed++; + if (!mode) mode = virXMLPropString(cur, "mode"); if (!haveTLS) diff --git a/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml b/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml new file mode 100644 index 0000000000..bb8592aa4f --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='tcp'> + <source mode='bind' host='localhost' service='5678'/> + <source mode='connect' host='localhost' service='5678'/> + <protocol type='raw'/> + <target type='virtio' name='test1'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml b/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml new file mode 100644 index 0000000000..2b87a1bfaa --- /dev/null +++ b/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='virtio-serial' index='0'/> + <channel type='udp'> + <source mode='connect' host='localhost' service='5678'/> + <source mode='bind' host='localhost' service='4567'/> + <source mode='connect' host='localhost' service='3456'/> + <target type='virtio' name='test1'/> + </channel> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 7dc137ed16..03913a68c9 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -110,9 +110,13 @@ mymain(void) TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_FULL("chardev-tcp-missing-service", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("chardev-tcp-multiple-source", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-udp"); DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FULL("chardev-udp-multiple-source", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-unix"); DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:05AM +0200, Pavel Hrdina wrote:
Currently we accept and correctly parse this chardev XML:
... <channel type='tcp'> <source mode='connect'/> <source mode='bind' host='localhost'/> <source service='4567'/> <target type='virtio' name='test'/> </channel> ...
The parsed formatted XML is:
... <channel type='tcp'> <source mode='connect' host='localhost' service='4567'/> <target type='virtio' name='test'/> </channel> ...
That behavior is super wrong and should not be allowed. If you notice the current parse takes the first found attribute and uses that value, so for example from the "<source mode='bind' host='localhost'/>" only the "host" attribute is used. It works the same way for all possible attributes that we are able to parse for source element.
This patch enforces providing only one source element for all character devices, only for UDP type we allow to provide two source elements since you can specify both modes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++ .../generic-chardev-tcp-multiple-source.xml | 26 ++++++++++++++++++++++ .../generic-chardev-udp-multiple-source.xml | 26 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 4 ++++ 4 files changed, 73 insertions(+) create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml
ACK Jan
Remove check whether a variable was already set because the element is parsed only once now. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 651a049cf1..639aa430ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10917,6 +10917,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *append = NULL; char *haveTLS = NULL; char *tlsFromConfig = NULL; + bool logParsed = false; int sourceParsed = 0; for (; cur; cur = cur->next) { @@ -11021,10 +11022,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, ctxt->node = saved_node; } } else if (virXMLNodeNameEqual(cur, "log")) { - if (!logfile) - logfile = virXMLPropString(cur, "file"); - if (!logappend) - logappend = virXMLPropString(cur, "append"); + if (logParsed) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one protocol element is allowed for " + "character device")); + goto error; + } + logParsed = true; + logfile = virXMLPropString(cur, "file"); + logappend = virXMLPropString(cur, "append"); } else if (virXMLNodeNameEqual(cur, "protocol")) { if (!protocol) protocol = virXMLPropString(cur, "type"); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:06AM +0200, Pavel Hrdina wrote:
Remove check whether a variable was already set because the element is parsed only once now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
ACK Jan
Remove check whether a variable was already set because the element is parsed only once now. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 639aa430ae..bb4be5d1cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10918,6 +10918,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *haveTLS = NULL; char *tlsFromConfig = NULL; bool logParsed = false; + bool protocolParsed = false; int sourceParsed = 0; for (; cur; cur = cur->next) { @@ -11032,8 +11033,14 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, logfile = virXMLPropString(cur, "file"); logappend = virXMLPropString(cur, "append"); } else if (virXMLNodeNameEqual(cur, "protocol")) { - if (!protocol) - protocol = virXMLPropString(cur, "type"); + if (protocolParsed) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one log element is allowed for " + "character device")); + goto error; + } + protocolParsed = true; + protocol = virXMLPropString(cur, "type"); } } -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:07AM +0200, Pavel Hrdina wrote:
Remove check whether a variable was already set because the element is parsed only once now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bb4be5d1cd..8fe79f70bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10885,6 +10885,30 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, return ret; } + +static int +virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, + xmlNodePtr protocol) +{ + char *prot = NULL; + + if (def->type != VIR_DOMAIN_CHR_TYPE_TCP) + return 0; + + if ((prot = virXMLPropString(protocol, "type")) && + (def->data.tcp.protocol = + virDomainChrTcpProtocolTypeFromString(prot)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown protocol '%s'"), prot); + VIR_FREE(prot); + return -1; + } + + VIR_FREE(prot); + return 0; +} + + #define SERIAL_CHANNEL_NAME_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." @@ -10910,7 +10934,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *logfile = NULL; char *logappend = NULL; char *mode = NULL; - char *protocol = NULL; char *channel = NULL; char *master = NULL; char *slave = NULL; @@ -11040,7 +11063,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } protocolParsed = true; - protocol = virXMLPropString(cur, "type"); + if (virDomainChrSourceDefParseProtocol(def, cur) < 0) + goto error; } } @@ -11151,16 +11175,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } def->data.tcp.tlsFromConfig = !!tmp; } - - if (!protocol) - def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; - else if ((def->data.tcp.protocol = - virDomainChrTcpProtocolTypeFromString(protocol)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown protocol '%s'"), protocol); - goto error; - } - break; case VIR_DOMAIN_CHR_TYPE_UDP: @@ -11227,7 +11241,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, ret = 0; cleanup: VIR_FREE(mode); - VIR_FREE(protocol); VIR_FREE(bindHost); VIR_FREE(bindService); VIR_FREE(connectHost); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:08AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bb4be5d1cd..8fe79f70bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11040,7 +11063,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } protocolParsed = true; - protocol = virXMLPropString(cur, "type"); + if (virDomainChrSourceDefParseProtocol(def, cur) < 0) + goto error; } }
@@ -11151,16 +11175,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } def->data.tcp.tlsFromConfig = !!tmp; } - - if (!protocol) - def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
This removes the explicit assignment of VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW if no protocol node has been seen. The most direct equivalent would be: if (!protocolParsed) but I would also be okay with (in the order of preference) 1. initializing it before we start parsing the node 2. adding a _DEFAULT enum value and changing it in PostParse 3. explicitly assigning VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW = 0 and letting calloc do the initialization ACK with that fixed Jan
- else if ((def->data.tcp.protocol = - virDomainChrTcpProtocolTypeFromString(protocol)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown protocol '%s'"), protocol); - goto error; - } - break;
case VIR_DOMAIN_CHR_TYPE_UDP:
On Tue, Aug 22, 2017 at 03:59:01PM +0200, Ján Tomko wrote:
On Mon, Aug 21, 2017 at 10:07:08AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bb4be5d1cd..8fe79f70bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11040,7 +11063,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } protocolParsed = true; - protocol = virXMLPropString(cur, "type"); + if (virDomainChrSourceDefParseProtocol(def, cur) < 0) + goto error; } }
@@ -11151,16 +11175,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } def->data.tcp.tlsFromConfig = !!tmp; } - - if (!protocol) - def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
This removes the explicit assignment of VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW if no protocol node has been seen.
The most direct equivalent would be: if (!protocolParsed) but I would also be okay with (in the order of preference) 1. initializing it before we start parsing the node
This would require to check for the chardev type and I'm not a fan of that.
2. adding a _DEFAULT enum value and changing it in PostParse
This is probably for a follow-up patch and the preferred way.
3. explicitly assigning VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW = 0 and letting calloc do the initialization
I'll go with this option since it's the easiest one :)
ACK with that fixed
Thanks, I'll push the series shortly. Pavel
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8fe79f70bf..eb576b90ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10909,6 +10909,28 @@ virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, } +static int +virDomainChrSourceDefParseLog(virDomainChrSourceDefPtr def, + xmlNodePtr log) +{ + char *append = NULL; + + def->logfile = virXMLPropString(log, "file"); + + if ((append = virXMLPropString(log, "append")) && + (def->logappend = virTristateSwitchTypeFromString(append)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid append attribute value '%s'"), + append); + VIR_FREE(append); + return -1; + } + + VIR_FREE(append); + return 0; +} + + #define SERIAL_CHANNEL_NAME_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." @@ -10931,8 +10953,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *connectHost = NULL; char *connectService = NULL; char *path = NULL; - char *logfile = NULL; - char *logappend = NULL; char *mode = NULL; char *channel = NULL; char *master = NULL; @@ -11053,8 +11073,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } logParsed = true; - logfile = virXMLPropString(cur, "file"); - logappend = virXMLPropString(cur, "append"); + if (virDomainChrSourceDefParseLog(def, cur) < 0) + goto error; } else if (virXMLNodeNameEqual(cur, "protocol")) { if (protocolParsed) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -11228,16 +11248,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; } - def->logfile = logfile; - logfile = NULL; - - if (logappend != NULL && - (def->logappend = virTristateSwitchTypeFromString(logappend)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), logappend); - goto error; - } - ret = 0; cleanup: VIR_FREE(mode); @@ -11248,8 +11258,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(path); VIR_FREE(channel); VIR_FREE(append); - VIR_FREE(logappend); - VIR_FREE(logfile); VIR_FREE(haveTLS); VIR_FREE(tlsFromConfig); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:09AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 60 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb576b90ae..e0045eba19 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10886,6 +10886,39 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, } +typedef enum { + VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT, + VIR_DOMAIN_CHR_SOURCE_MODE_BIND, +} virDomainChrSourceModeType; + + +/** + * virDomainChrSourceDefParseMode: + * @source: XML dom node + * + * Returns: -1 in case of error, + * virDomainChrSourceModeType in case of success + */ +static int +virDomainChrSourceDefParseMode(xmlNodePtr source) +{ + char *mode = virXMLPropString(source, "mode"); + int ret = -1; + + if (!mode || STREQ(mode, "connect")) { + ret = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT; + } else if (STREQ(mode, "bind")) { + ret = VIR_DOMAIN_CHR_SOURCE_MODE_BIND; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown source mode '%s'"), mode); + } + + VIR_FREE(mode); + return ret; +} + + static int virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, xmlNodePtr protocol) @@ -10948,12 +10981,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, int nvmSeclabels) { int ret = -1; + int mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT; char *bindHost = NULL; char *bindService = NULL; char *connectHost = NULL; char *connectService = NULL; char *path = NULL; - char *mode = NULL; char *channel = NULL; char *master = NULL; char *slave = NULL; @@ -10985,8 +11018,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } sourceParsed++; - if (!mode) - mode = virXMLPropString(cur, "mode"); if (!haveTLS) haveTLS = virXMLPropString(cur, "tls"); if (!tlsFromConfig) @@ -11005,29 +11036,29 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, (def->type != VIR_DOMAIN_CHR_TYPE_PTY || !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))) path = virXMLPropString(cur, "path"); + if (def->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + if ((mode = virDomainChrSourceDefParseMode(cur)) < 0) + goto error; + } break; case VIR_DOMAIN_CHR_TYPE_UDP: case VIR_DOMAIN_CHR_TYPE_TCP: - if (!mode || STREQ(mode, "connect")) { + if ((mode = virDomainChrSourceDefParseMode(cur)) < 0) + goto error; + if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT) { if (!connectHost) connectHost = virXMLPropString(cur, "host"); if (!connectService) connectService = virXMLPropString(cur, "service"); - } else if (STREQ(mode, "bind")) { + } else if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND) { if (!bindHost) bindHost = virXMLPropString(cur, "host"); if (!bindService) bindService = virXMLPropString(cur, "service"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown source mode '%s'"), mode); - goto error; } - if (def->type == VIR_DOMAIN_CHR_TYPE_UDP) - VIR_FREE(mode); break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: @@ -11137,7 +11168,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_TCP: - if (!mode || STREQ(mode, "connect")) { + if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT) { if (!connectHost) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source host attribute for char device")); @@ -11155,7 +11186,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.tcp.service = connectService; connectService = NULL; def->data.tcp.listen = false; - } else { + } else if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND) { if (!bindHost) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source host attribute for char device")); @@ -11226,7 +11257,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } - def->data.nix.listen = mode != NULL && STRNEQ(mode, "connect"); + def->data.nix.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND; def->data.nix.path = path; path = NULL; @@ -11250,7 +11281,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, ret = 0; cleanup: - VIR_FREE(mode); VIR_FREE(bindHost); VIR_FREE(bindService); VIR_FREE(connectHost); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:10AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 60 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 15 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 121 +++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 64 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e0045eba19..023d86cec5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10920,6 +10920,52 @@ virDomainChrSourceDefParseMode(xmlNodePtr source) static int +virDomainChrSourceDefParseTCP(virDomainChrSourceDefPtr def, + xmlNodePtr source, + unsigned int flags) +{ + int mode; + char *tmp = NULL; + int tmpVal; + + if ((mode = virDomainChrSourceDefParseMode(source)) < 0) + goto error; + + def->data.tcp.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND; + def->data.tcp.host = virXMLPropString(source, "host"); + def->data.tcp.service = virXMLPropString(source, "service"); + + if ((tmp = virXMLPropString(source, "tls"))) { + if ((def->data.tcp.haveTLS = virTristateBoolTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown chardev 'tls' setting '%s'"), + tmp); + goto error; + } + VIR_FREE(tmp); + } + + if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) && + (tmp = virXMLPropString(source, "tlsFromConfig"))) { + if (virStrToLong_i(tmp, NULL, 10, &tmpVal) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid tlsFromConfig value: %s"), + tmp); + goto error; + } + def->data.tcp.tlsFromConfig = !!tmpVal; + VIR_FREE(tmp); + } + + return 0; + + error: + VIR_FREE(tmp); + return -1; +} + + +static int virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, xmlNodePtr protocol) { @@ -10991,8 +11037,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *master = NULL; char *slave = NULL; char *append = NULL; - char *haveTLS = NULL; - char *tlsFromConfig = NULL; bool logParsed = false; bool protocolParsed = false; int sourceParsed = 0; @@ -11018,11 +11062,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } sourceParsed++; - if (!haveTLS) - haveTLS = virXMLPropString(cur, "tls"); - if (!tlsFromConfig) - tlsFromConfig = virXMLPropString(cur, "tlsFromConfig"); - switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PTY: @@ -11044,7 +11083,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_UDP: - case VIR_DOMAIN_CHR_TYPE_TCP: if ((mode = virDomainChrSourceDefParseMode(cur)) < 0) goto error; if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT) { @@ -11058,7 +11096,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, if (!bindService) bindService = virXMLPropString(cur, "service"); } + break; + case VIR_DOMAIN_CHR_TYPE_TCP: + if (virDomainChrSourceDefParseTCP(def, cur, flags) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: @@ -11168,63 +11210,16 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_TCP: - if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT) { - if (!connectHost) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source host attribute for char device")); - goto error; - } - - if (!connectService) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - goto error; - } - - def->data.tcp.host = connectHost; - connectHost = NULL; - def->data.tcp.service = connectService; - connectService = NULL; - def->data.tcp.listen = false; - } else if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND) { - if (!bindHost) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source host attribute for char device")); - goto error; - } - - if (!bindService) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - goto error; - } - - def->data.tcp.host = bindHost; - bindHost = NULL; - def->data.tcp.service = bindService; - bindService = NULL; - def->data.tcp.listen = true; - } - - if (haveTLS && - (def->data.tcp.haveTLS = - virTristateBoolTypeFromString(haveTLS)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown chardev 'tls' setting '%s'"), - haveTLS); + if (!def->data.tcp.host) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source host attribute for char device")); goto error; } - if (tlsFromConfig && - flags & VIR_DOMAIN_DEF_PARSE_STATUS) { - int tmp; - if (virStrToLong_i(tlsFromConfig, NULL, 10, &tmp) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid tlsFromConfig value: %s"), - tlsFromConfig); - goto error; - } - def->data.tcp.tlsFromConfig = !!tmp; + if (!def->data.tcp.service) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source service attribute for char device")); + goto error; } break; @@ -11288,8 +11283,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(path); VIR_FREE(channel); VIR_FREE(append); - VIR_FREE(haveTLS); - VIR_FREE(tlsFromConfig); return ret; -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:11AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 121 +++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 64 deletions(-)
ACK Jan
The extra check whether (connect|bind)(Host|Service) was set is required because for UDP chardev there can be two source elements. Without the check there could be a memory leak. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 56 ++++++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 023d86cec5..d8a195a141 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10966,6 +10966,29 @@ virDomainChrSourceDefParseTCP(virDomainChrSourceDefPtr def, static int +virDomainChrSourceDefParseUDP(virDomainChrSourceDefPtr def, + xmlNodePtr source) +{ + int mode; + + if ((mode = virDomainChrSourceDefParseMode(source)) < 0) + return -1; + + if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT && + !def->data.udp.connectHost && !def->data.udp.connectService) { + def->data.udp.connectHost = virXMLPropString(source, "host"); + def->data.udp.connectService = virXMLPropString(source, "service"); + } else if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND && + !def->data.udp.bindHost && !def->data.udp.bindService) { + def->data.udp.bindHost = virXMLPropString(source, "host"); + def->data.udp.bindService = virXMLPropString(source, "service"); + } + + return 0; +} + + +static int virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, xmlNodePtr protocol) { @@ -11028,10 +11051,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, { int ret = -1; int mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT; - char *bindHost = NULL; - char *bindService = NULL; - char *connectHost = NULL; - char *connectService = NULL; char *path = NULL; char *channel = NULL; char *master = NULL; @@ -11083,19 +11102,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_UDP: - if ((mode = virDomainChrSourceDefParseMode(cur)) < 0) + if (virDomainChrSourceDefParseUDP(def, cur) < 0) goto error; - if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT) { - if (!connectHost) - connectHost = virXMLPropString(cur, "host"); - if (!connectService) - connectService = virXMLPropString(cur, "service"); - } else if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND) { - if (!bindHost) - bindHost = virXMLPropString(cur, "host"); - if (!bindService) - bindService = virXMLPropString(cur, "service"); - } break; case VIR_DOMAIN_CHR_TYPE_TCP: @@ -11224,21 +11232,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_UDP: - if (!connectService) { + if (!def->data.udp.connectService) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source service attribute for char device")); goto error; } - - def->data.udp.connectHost = connectHost; - connectHost = NULL; - def->data.udp.connectService = connectService; - connectService = NULL; - - def->data.udp.bindHost = bindHost; - bindHost = NULL; - def->data.udp.bindService = bindService; - bindService = NULL; break; case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -11276,10 +11274,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, ret = 0; cleanup: - VIR_FREE(bindHost); - VIR_FREE(bindService); - VIR_FREE(connectHost); - VIR_FREE(connectService); VIR_FREE(path); VIR_FREE(channel); VIR_FREE(append); -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:12AM +0200, Pavel Hrdina wrote:
The extra check whether (connect|bind)(Host|Service) was set is required because for UDP chardev there can be two source elements. Without the check there could be a memory leak.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 56 ++++++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 31 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8a195a141..d69590e29d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10989,6 +10989,23 @@ virDomainChrSourceDefParseUDP(virDomainChrSourceDefPtr def, static int +virDomainChrSourceDefParseUnix(virDomainChrSourceDefPtr def, + xmlNodePtr source) +{ + + int mode; + + if ((mode = virDomainChrSourceDefParseMode(source)) < 0) + return -1; + + def->data.nix.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND; + def->data.nix.path = virXMLPropString(source, "path"); + + return 0; +} + + +static int virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, xmlNodePtr protocol) { @@ -11050,7 +11067,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, int nvmSeclabels) { int ret = -1; - int mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT; char *path = NULL; char *channel = NULL; char *master = NULL; @@ -11086,7 +11102,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: - case VIR_DOMAIN_CHR_TYPE_UNIX: if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) append = virXMLPropString(cur, "append"); /* PTY path is only parsed from live xml. */ @@ -11094,11 +11109,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, (def->type != VIR_DOMAIN_CHR_TYPE_PTY || !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))) path = virXMLPropString(cur, "path"); - if (def->type == VIR_DOMAIN_CHR_TYPE_UNIX) { - if ((mode = virDomainChrSourceDefParseMode(cur)) < 0) - goto error; - } + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (virDomainChrSourceDefParseUnix(def, cur) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_UDP: @@ -11241,7 +11256,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, case VIR_DOMAIN_CHR_TYPE_UNIX: /* path can be auto generated */ - if (!path && + if (!def->data.nix.path && (!chr_def || (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { @@ -11249,11 +11264,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, _("Missing source path attribute for char device")); goto error; } - - def->data.nix.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND; - - def->data.nix.path = path; - path = NULL; break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:13AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)
ACK Jan
Since the source element is parsed only once for these type of character devices we don't have to use temporary variable and check whether the variable was already set. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 43 ++++++++++++------------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d69590e29d..84184a265e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11067,10 +11067,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, int nvmSeclabels) { int ret = -1; - char *path = NULL; - char *channel = NULL; - char *master = NULL; - char *slave = NULL; char *append = NULL; bool logParsed = false; bool protocolParsed = false; @@ -11105,10 +11101,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) append = virXMLPropString(cur, "append"); /* PTY path is only parsed from live xml. */ - if (!path && - (def->type != VIR_DOMAIN_CHR_TYPE_PTY || - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))) - path = virXMLPropString(cur, "path"); + if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || + !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) + def->data.file.path = virXMLPropString(cur, "path"); break; case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -11127,15 +11122,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - if (!channel) - channel = virXMLPropString(cur, "channel"); + def->data.spiceport.channel = virXMLPropString(cur, "channel"); break; case VIR_DOMAIN_CHR_TYPE_NMDM: - if (!master) - master = virXMLPropString(cur, "master"); - if (!slave) - slave = virXMLPropString(cur, "slave"); + def->data.nmdm.master = virXMLPropString(cur, "master"); + def->data.nmdm.slave = virXMLPropString(cur, "slave"); break; case VIR_DOMAIN_CHR_TYPE_LAST: @@ -11202,34 +11194,26 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, _("Invalid append attribute value '%s'"), append); goto error; } - if (!path && + if (!def->data.file.path && def->type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; } - - def->data.file.path = path; - path = NULL; break; case VIR_DOMAIN_CHR_TYPE_NMDM: - if (!master) { + if (!def->data.nmdm.master) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing master path attribute for nmdm device")); goto error; } - if (!slave) { + if (!def->data.nmdm.slave) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing slave path attribute for nmdm device")); goto error; } - - def->data.nmdm.master = master; - def->data.nmdm.slave = slave; - master = NULL; - slave = NULL; break; case VIR_DOMAIN_CHR_TYPE_TCP: @@ -11267,25 +11251,22 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - if (!channel) { + if (!def->data.spiceport.channel) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing source channel attribute for char device")); goto error; } - if (strspn(channel, SERIAL_CHANNEL_NAME_CHARS) < strlen(channel)) { + if (strspn(def->data.spiceport.channel, + SERIAL_CHANNEL_NAME_CHARS) < strlen(def->data.spiceport.channel)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Invalid character in source channel for char device")); goto error; } - def->data.spiceport.channel = channel; - channel = NULL; break; } ret = 0; cleanup: - VIR_FREE(path); - VIR_FREE(channel); VIR_FREE(append); return ret; -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:14AM +0200, Pavel Hrdina wrote:
Since the source element is parsed only once for these type of character devices we don't have to use temporary variable and check whether the variable was already set.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 43 ++++++++++++------------------------------- 1 file changed, 12 insertions(+), 31 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 84184a265e..1baf6b9174 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11006,6 +11006,28 @@ virDomainChrSourceDefParseUnix(virDomainChrSourceDefPtr def, static int +virDomainChrSourceDefParseFile(virDomainChrSourceDefPtr def, + xmlNodePtr source) +{ + char *append = NULL; + + def->data.file.path = virXMLPropString(source, "path"); + + if ((append = virXMLPropString(source, "append")) && + (def->data.file.append = virTristateSwitchTypeFromString(append)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid append attribute value '%s'"), + append); + VIR_FREE(append); + return -1; + } + + VIR_FREE(append); + return 0; +} + + +static int virDomainChrSourceDefParseProtocol(virDomainChrSourceDefPtr def, xmlNodePtr protocol) { @@ -11067,7 +11089,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, int nvmSeclabels) { int ret = -1; - char *append = NULL; bool logParsed = false; bool protocolParsed = false; int sourceParsed = 0; @@ -11095,11 +11116,13 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: + if (virDomainChrSourceDefParseFile(def, cur) < 0) + goto error; + break; + case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: - if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) - append = virXMLPropString(cur, "append"); /* PTY path is only parsed from live xml. */ if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) @@ -11188,12 +11211,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: - if (append && def->type == VIR_DOMAIN_CHR_TYPE_FILE && - (def->data.file.append = virTristateSwitchTypeFromString(append)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), append); - goto error; - } if (!def->data.file.path && def->type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -11267,8 +11284,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, ret = 0; cleanup: - VIR_FREE(append); - return ret; error: -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:15AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)
ACK Jan
There is no reason why to share the same code for PTY and other file based chardev source types. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1baf6b9174..3e20b22799 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11121,14 +11121,16 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_PTY: - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_PIPE: /* PTY path is only parsed from live xml. */ - if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) def->data.file.path = virXMLPropString(cur, "path"); break; + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + def->data.file.path = virXMLPropString(cur, "path"); + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: if (virDomainChrSourceDefParseUnix(def, cur) < 0) goto error; @@ -11201,6 +11203,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_SPICEVMC: @@ -11208,11 +11211,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_FILE: - case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: - if (!def->data.file.path && - def->type != VIR_DOMAIN_CHR_TYPE_PTY) { + if (!def->data.file.path) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:16AM +0200, Pavel Hrdina wrote:
There is no reason why to share the same code for PTY and other file based chardev source types.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
ACK Jan
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 221 +++++++++++++++++++++++++++++-------------------- 1 file changed, 132 insertions(+), 89 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e20b22799..b698569f82 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5034,6 +5034,101 @@ virDomainDefHasUSB(const virDomainDef *def) return false; } + +#define SERIAL_CHANNEL_NAME_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." + + +static int +virDomainChrSourceDefValidate(const virDomainChrSourceDef *def, + const virDomainChrDef *chr_def) +{ + switch ((virDomainChrType) def->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + if (!def->data.file.path) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source path attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_NMDM: + if (!def->data.nmdm.master) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing master path attribute for nmdm device")); + return -1; + } + + if (!def->data.nmdm.slave) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing slave path attribute for nmdm device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_TCP: + if (!def->data.tcp.host) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source host attribute for char device")); + return -1; + } + + if (!def->data.tcp.service) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source service attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UDP: + if (!def->data.udp.connectService) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source service attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: + /* path can be auto generated */ + if (!def->data.nix.path && + (!chr_def || + (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && + chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source path attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!def->data.spiceport.channel) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing source channel attribute for char device")); + return -1; + } + if (strspn(def->data.spiceport.channel, + SERIAL_CHANNEL_NAME_CHARS) < strlen(def->data.spiceport.channel)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Invalid character in source channel for char device")); + return -1; + } + break; + } + + return 0; +} + + static int virDomainRedirdevDefValidate(const virDomainDef *def, const virDomainRedirdevDef *redirdev) @@ -5046,7 +5141,7 @@ virDomainRedirdevDefValidate(const virDomainDef *def, return -1; } - return 0; + return virDomainChrSourceDefValidate(redirdev->source, NULL); } @@ -5083,6 +5178,33 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) static int +virDomainChrDefValidate(const virDomainChrDef *chr) +{ + return virDomainChrSourceDefValidate(chr->source, chr); +} + + +static int +virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard) +{ + if (smartcard->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) + return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL); + + return 0; +} + + +static int +virDomainRNGDefValidate(const virDomainRNGDef *rng) +{ + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + return virDomainChrSourceDefValidate(rng->source.chardev, NULL); + + return 0; +} + + +static int virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, const virDomainDef *def) { @@ -5099,6 +5221,15 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_CONTROLLER: return virDomainControllerDefValidate(dev->data.controller); + case VIR_DOMAIN_DEVICE_CHR: + return virDomainChrDefValidate(dev->data.chr); + + case VIR_DOMAIN_DEVICE_SMARTCARD: + return virDomainSmartcardDefValidate(dev->data.smartcard); + + case VIR_DOMAIN_DEVICE_RNG: + return virDomainRNGDefValidate(dev->data.rng); + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: @@ -5108,11 +5239,8 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_RNG: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: @@ -11072,9 +11200,6 @@ virDomainChrSourceDefParseLog(virDomainChrSourceDefPtr def, } -#define SERIAL_CHANNEL_NAME_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." - /* Parse the source half of the XML definition for a character device, * where node is the first element of node->children of the parent * element. def->type must already be valid. @@ -11201,88 +11326,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } } - switch ((virDomainChrType) def->type) { - case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_PTY: - case VIR_DOMAIN_CHR_TYPE_VC: - case VIR_DOMAIN_CHR_TYPE_STDIO: - case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - case VIR_DOMAIN_CHR_TYPE_LAST: - break; - - case VIR_DOMAIN_CHR_TYPE_FILE: - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_PIPE: - if (!def->data.file.path) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source path attribute for char device")); - goto error; - } - break; - - case VIR_DOMAIN_CHR_TYPE_NMDM: - if (!def->data.nmdm.master) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing master path attribute for nmdm device")); - goto error; - } - - if (!def->data.nmdm.slave) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing slave path attribute for nmdm device")); - goto error; - } - break; - - case VIR_DOMAIN_CHR_TYPE_TCP: - if (!def->data.tcp.host) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source host attribute for char device")); - goto error; - } - - if (!def->data.tcp.service) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - goto error; - } - break; - - case VIR_DOMAIN_CHR_TYPE_UDP: - if (!def->data.udp.connectService) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - goto error; - } - break; - - case VIR_DOMAIN_CHR_TYPE_UNIX: - /* path can be auto generated */ - if (!def->data.nix.path && - (!chr_def || - (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && - chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source path attribute for char device")); - goto error; - } - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - if (!def->data.spiceport.channel) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing source channel attribute for char device")); - goto error; - } - if (strspn(def->data.spiceport.channel, - SERIAL_CHANNEL_NAME_CHARS) < strlen(def->data.spiceport.channel)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Invalid character in source channel for char device")); - goto error; - } - break; - } - ret = 0; cleanup: return ret; -- 2.13.5
On Mon, Aug 21, 2017 at 10:07:17AM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 221 +++++++++++++++++++++++++++++-------------------- 1 file changed, 132 insertions(+), 89 deletions(-)
ACK Jan
participants (2)
-
Ján Tomko -
Pavel Hrdina