[libvirt] [PATCH 0/3] Misc adjustments from recent code review
The following were all part of the review of the TCP chardev TLS series which were outside the realm of the specific changes for the series... http://www.redhat.com/archives/libvir-list/2016-October/msg00742.html 1. Removal of cfg from qemuProcessPrepareDomain should be separate patch 2. Setting chardevTLSx509verify should have been it's own patch... 3. !conn in qemuDomainSecretChardevPrepare not necessary - so that's true for the SecretDiskPrepare and SecretHostdevPrepare too John Ferlan (3): qemu: Remove unnecessary cfg fetch/unref qemu: Add 'verify-peer=yes' test for chardev TCP TLS qemu: Remove unnecessary NULL arg check src/qemu/qemu_domain.c | 5 +-- src/qemu/qemu_process.c | 2 -- ...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 33 +++++++++++++++++ ...uxml2argv-serial-tcp-tlsx509-chardev-verify.xml | 41 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml -- 2.7.4
qemuProcessPrepareDomain has no need to fetch/unref the cfg, so remove it. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_process.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0f5a11b..d641f33 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn, size_t i; char *nodeset = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -5188,7 +5187,6 @@ qemuProcessPrepareDomain(virConnectPtr conn, cleanup: VIR_FREE(nodeset); virObjectUnref(caps); - virObjectUnref(cfg); return ret; } -- 2.7.4
Missing the option to set verify-peer to yes Signed-off-by: John Ferlan <jferlan@redhat.com> --- ...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 33 +++++++++++++++++ ...uxml2argv-serial-tcp-tlsx509-chardev-verify.xml | 41 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 3 files changed, 79 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args new file mode 100644 index 0000000..f521e33 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\ +localport=1111 \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\ +endpoint=client,verify-peer=yes \ +-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\ +tls-creds=objserial1_tls0 \ +-device isa-serial,chardev=charserial1,id=serial1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml new file mode 100644 index 0000000..1618b02 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml @@ -0,0 +1,41 @@ +<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</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <serial type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target port='0'/> + </serial> + <serial type='tcp'> + <source mode='connect' host='127.0.0.1' service='5555'/> + <protocol type='raw'/> + <target port='0'/> + </serial> + <console type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='serial' port='0'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b1cc4d8..3e9f825 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1162,6 +1162,11 @@ mymain(void) DO_TEST("serial-tcp-tlsx509-chardev", QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_TLS_CREDS_X509); + driver.config->chardevTLSx509verify = 1; + DO_TEST("serial-tcp-tlsx509-chardev-verify", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_OBJECT_TLS_CREDS_X509); + driver.config->chardevTLSx509verify = 0; driver.config->chardevTLS = 0; VIR_FREE(driver.config->chardevTLSx509certdir); DO_TEST("serial-many-chardev", -- 2.7.4
qemuDomainSecret{Disk|Hostdev}Prepare has a prototype that checks for ATTRIBUTE_NONNULL(1) for 'conn'. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c9416f..5e9f08f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1062,9 +1062,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = NULL; - if (!conn) - return 0; - if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; @@ -1147,7 +1144,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; qemuDomainSecretInfoPtr secinfo = NULL; - if (conn && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; -- 2.7.4
On Mon, Oct 17, 2016 at 03:48:53PM -0400, John Ferlan wrote:
The following were all part of the review of the TCP chardev TLS series which were outside the realm of the specific changes for the series...
http://www.redhat.com/archives/libvir-list/2016-October/msg00742.html
1. Removal of cfg from qemuProcessPrepareDomain should be separate patch 2. Setting chardevTLSx509verify should have been it's own patch... 3. !conn in qemuDomainSecretChardevPrepare not necessary - so that's true for the SecretDiskPrepare and SecretHostdevPrepare too
ACK series Pavel
On 18.10.2016 03:48, John Ferlan wrote:
The following were all part of the review of the TCP chardev TLS series which were outside the realm of the specific changes for the series...
http://www.redhat.com/archives/libvir-list/2016-October/msg00742.html
1. Removal of cfg from qemuProcessPrepareDomain should be separate patch 2. Setting chardevTLSx509verify should have been it's own patch... 3. !conn in qemuDomainSecretChardevPrepare not necessary - so that's true for the SecretDiskPrepare and SecretHostdevPrepare too
John Ferlan (3): qemu: Remove unnecessary cfg fetch/unref qemu: Add 'verify-peer=yes' test for chardev TCP TLS qemu: Remove unnecessary NULL arg check
src/qemu/qemu_domain.c | 5 +-- src/qemu/qemu_process.c | 2 -- ...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 33 +++++++++++++++++ ...uxml2argv-serial-tcp-tlsx509-chardev-verify.xml | 41 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.xml
ACK series Michal
participants (3)
-
John Ferlan -
Michal Privoznik -
Pavel Hrdina