[libvirt] [PATCH v2 0/9] qemu: Make it possible to run domains with dirrefent seclabels
We offer setting seclabel for the whole domain, but we never fixed the fact that the domain will not be able to even create its monitor socket because the directory is owned by the default preconfigured user and group. Moreover the selinux context can be off as well. So this patch series fixes few preliminary problems and then changes autogenerating so that it creates path per-domain. That way we can start do mains with any seclabels we want without that annoying error message (or similar ones, depends on your configuration): error: Failed to start domain dummy error: internal error: process exited while connecting to monitor: 2015-08-13T15:26:01.474941Z qemu-system-x86_64: -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/dummy.monitor,server,nowait: Failed to unlink socket /var/lib/libvirt/qemu/dummy.monitor: Permission denied The idea is mentioned in this thread in which I tried fixing it pretty badly without thinking it through (feel free to read the patch for your amusement): https://www.redhat.com/archives/libvir-list/2015-February/msg01051.htmlA One thing to note here is that tests for patch 7/9 are in a separate patch 8/9 and only minimal. We can also use qemuxml2argvtest to test for the same thing and indeed that is patch 9/9 that I haven't sent. I will send it if anyone wants to have that been done as well, but I believe the qemuxml2argvtest can be left as it currently is because the patch itself has around 350KiB. It's also enough if you just tell me in the review that I should squash it in the previous commit (which I don't suppose anyone will do). Or another idea, I made it available on my github: https://github.com/nertpinx/libvirt.git (branch bz1146886) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886 Martin Kletzander (9): security_selinux: Use proper structure to access socket data security_dac: Label non-listening sockets security: Add virSecurityDomainSetDirLabel security_stack: Add SetDirLabel support security_selinux: Add SetDirLabel support security_dac: Add SetDirLabel support qemu: Fix access to auto-generated socket paths tests: Use qemuProcessPrepareMonitorChr in qemuxmlnstest tests: Use qemuProcessPrepareMonitorChr in qemuxml2argvtest src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 +++--- src/qemu/qemu_process.c | 57 +++++++++++++++++++++- src/security/security_dac.c | 30 +++++++++++- src/security/security_driver.h | 5 ++ src/security/security_manager.c | 17 +++++++ src/security/security_manager.h | 4 ++ src/security/security_selinux.c | 19 +++++++- src/security/security_stack.c | 20 ++++++++ .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args | 3 +- .../qemuxml2argv-aarch64-cpu-passthrough.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-aarch64-gic.args | 3 +- .../qemuxml2argv-aarch64-kvm-32-on-64.args | 2 +- .../qemuxml2argv-aarch64-virt-default-nic.args | 3 +- .../qemuxml2argv-aarch64-virt-virtio.args | 3 +- .../qemuxml2argv-arm-vexpressa9-basic.args | 3 +- .../qemuxml2argv-arm-vexpressa9-nodevs.args | 3 +- .../qemuxml2argv-arm-vexpressa9-virtio.args | 3 +- .../qemuxml2argv-arm-virt-virtio.args | 3 +- .../qemuxml2argv-balloon-device-auto.args | 3 +- .../qemuxml2argv-balloon-device-period.args | 3 +- .../qemuxml2argv-balloon-device.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-bios.args | 3 +- .../qemuxml2argv-blkdeviotune-max.args | 3 +- .../qemuxml2argv-blkdeviotune.args | 3 +- .../qemuxml2argv-blkiotune-device.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-blkiotune.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-boot-cdrom.args | 3 +- .../qemuxml2argv-boot-complex-bootindex.args | 2 +- .../qemuxml2argv-boot-complex.args | 2 +- .../qemuxml2argv-boot-floppy-q35.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.args | 3 +- ...xml2argv-boot-menu-disable-drive-bootindex.args | 2 +- .../qemuxml2argv-boot-menu-disable-drive.args | 2 +- .../qemuxml2argv-boot-menu-disable.args | 3 +- ...qemuxml2argv-boot-menu-enable-with-timeout.args | 2 +- .../qemuxml2argv-boot-menu-enable.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-boot-multi.args | 3 +- .../qemuxml2argv-boot-network.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-boot-order.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-boot-strict.args | 2 +- .../qemuxml2argv-bootindex-floppy-q35.args | 2 +- .../qemuxml2argv-channel-guestfwd.args | 3 +- .../qemuxml2argv-channel-spicevmc-old.args | 3 +- .../qemuxml2argv-channel-spicevmc.args | 3 +- .../qemuxml2argv-channel-virtio-auto.args | 2 +- .../qemuxml2argv-channel-virtio-autoadd.args | 2 +- .../qemuxml2argv-channel-virtio-autoassign.args | 2 +- .../qemuxml2argv-channel-virtio-default.args | 2 +- .../qemuxml2argv-channel-virtio-state.args | 2 +- .../qemuxml2argv-channel-virtio-unix.args | 9 ++-- .../qemuxml2argv-channel-virtio.args | 2 +- .../qemuxml2argv-clock-catchup.args | 3 +- .../qemuxml2argv-clock-france.args | 3 +- .../qemuxml2argv-clock-hpet-off.args | 3 +- ...muxml2argv-clock-localtime-basis-localtime.args | 3 +- .../qemuxml2argv-clock-localtime.args | 3 +- .../qemuxml2argv-clock-timer-hyperv-rtc.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args | 2 +- .../qemuxml2argv-clock-variable.args | 3 +- .../qemuxml2argv-console-compat-auto.args | 3 +- .../qemuxml2argv-console-compat-chardev.args | 3 +- .../qemuxml2argv-console-compat.args | 3 +- .../qemuxml2argv-console-sclp.args | 2 +- .../qemuxml2argv-console-virtio-ccw.args | 2 +- .../qemuxml2argv-console-virtio-many.args | 2 +- .../qemuxml2argv-console-virtio-s390.args | 2 +- .../qemuxml2argv-console-virtio.args | 2 +- .../qemuxml2argv-controller-order.args | 2 +- .../qemuxml2argv-cpu-Haswell-noTSX.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-cpu-Haswell.args | 3 +- .../qemuxml2argv-cpu-Haswell2.args | 3 +- .../qemuxml2argv-cpu-Haswell3.args | 3 +- .../qemuxml2argv-cpu-eoi-disabled.args | 3 +- .../qemuxml2argv-cpu-eoi-enabled.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-cpu-exact1.args | 3 +- .../qemuxml2argv-cpu-exact2-nofallback.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args | 3 +- .../qemuxml2argv-cpu-fallback.args | 2 +- .../qemuxml2argv-cpu-host-kvmclock.args | 3 +- .../qemuxml2argv-cpu-host-model-fallback.args | 2 +- .../qemuxml2argv-cpu-host-model-vendor.args | 2 +- .../qemuxml2argv-cpu-host-model.args | 2 +- ...qemuxml2argv-cpu-host-passthrough-features.args | 2 +- .../qemuxml2argv-cpu-host-passthrough.args | 2 +- .../qemuxml2argv-cpu-kvmclock.args | 3 +- .../qemuxml2argv-cpu-minimum1.args | 3 +- .../qemuxml2argv-cpu-minimum2.args | 3 +- .../qemuxml2argv-cpu-numa-disjoint.args | 3 +- .../qemuxml2argv-cpu-numa-no-memory-element.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args | 3 +- .../qemuxml2argv-cpu-topology1.args | 3 +- .../qemuxml2argv-cpu-topology2.args | 3 +- .../qemuxml2argv-cpu-topology3.args | 3 +- .../qemuxml2argv-cputune-numatune.args | 2 +- .../qemuxml2argv-cputune-zero-shares.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-cputune.args | 3 +- .../qemuxml2argv-default-kvm-host-arch.args | 3 +- .../qemuxml2argv-default-qemu-host-arch.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-disk-aio.args | 3 +- .../qemuxml2argv-disk-blockio.args | 3 +- .../qemuxml2argv-disk-cdrom-empty.args | 3 +- .../qemuxml2argv-disk-cdrom-network-ftp.args | 3 +- .../qemuxml2argv-disk-cdrom-network-ftps.args | 3 +- .../qemuxml2argv-disk-cdrom-network-http.args | 3 +- .../qemuxml2argv-disk-cdrom-network-https.args | 3 +- .../qemuxml2argv-disk-cdrom-network-tftp.args | 3 +- ...qemuxml2argv-disk-cdrom-tray-no-device-cap.args | 3 +- .../qemuxml2argv-disk-cdrom-tray.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-cdrom.args | 3 +- .../qemuxml2argv-disk-copy_on_read.args | 2 +- .../qemuxml2argv-disk-drive-boot-cdrom.args | 3 +- .../qemuxml2argv-disk-drive-boot-disk.args | 3 +- .../qemuxml2argv-disk-drive-cache-directsync.args | 3 +- .../qemuxml2argv-disk-drive-cache-unsafe.args | 3 +- .../qemuxml2argv-disk-drive-cache-v1-none.args | 3 +- .../qemuxml2argv-disk-drive-cache-v1-wb.args | 3 +- .../qemuxml2argv-disk-drive-cache-v1-wt.args | 3 +- .../qemuxml2argv-disk-drive-cache-v2-none.args | 3 +- .../qemuxml2argv-disk-drive-cache-v2-wb.args | 3 +- .../qemuxml2argv-disk-drive-cache-v2-wt.args | 3 +- .../qemuxml2argv-disk-drive-copy-on-read.args | 3 +- .../qemuxml2argv-disk-drive-discard.args | 3 +- ...uxml2argv-disk-drive-error-policy-enospace.args | 2 +- .../qemuxml2argv-disk-drive-error-policy-stop.args | 2 +- ...gv-disk-drive-error-policy-wreport-rignore.args | 2 +- .../qemuxml2argv-disk-drive-fat.args | 3 +- .../qemuxml2argv-disk-drive-fmt-qcow.args | 3 +- .../qemuxml2argv-disk-drive-network-gluster.args | 3 +- ...qemuxml2argv-disk-drive-network-iscsi-auth.args | 3 +- .../qemuxml2argv-disk-drive-network-iscsi-lun.args | 3 +- .../qemuxml2argv-disk-drive-network-iscsi.args | 3 +- ...qemuxml2argv-disk-drive-network-nbd-export.args | 3 +- ...ml2argv-disk-drive-network-nbd-ipv6-export.args | 3 +- .../qemuxml2argv-disk-drive-network-nbd-ipv6.args | 3 +- .../qemuxml2argv-disk-drive-network-nbd-unix.args | 3 +- .../qemuxml2argv-disk-drive-network-nbd.args | 3 +- .../qemuxml2argv-disk-drive-network-rbd-auth.args | 3 +- ...muxml2argv-disk-drive-network-rbd-ceph-env.args | 3 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 3 +- .../qemuxml2argv-disk-drive-network-rbd.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-no-boot.args | 2 +- .../qemuxml2argv-disk-drive-readonly-disk.args | 3 +- ...qemuxml2argv-disk-drive-readonly-no-device.args | 3 +- .../qemuxml2argv-disk-drive-shared.args | 3 +- .../qemuxml2argv-disk-floppy-pseries.args | 3 +- ...emuxml2argv-disk-floppy-tray-no-device-cap.args | 3 +- .../qemuxml2argv-disk-floppy-tray.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-floppy.args | 3 +- .../qemuxml2argv-disk-geometry.args | 3 +- .../qemuxml2argv-disk-ide-drive-split.args | 3 +- .../qemuxml2argv-disk-ide-wwn.args | 3 +- .../qemuxml2argv-disk-ioeventfd.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-iscsi.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-disk-many.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-order.args | 2 +- .../qemuxml2argv-disk-sata-device.args | 3 +- .../qemuxml2argv-disk-scsi-device-auto.args | 3 +- .../qemuxml2argv-disk-scsi-device.args | 3 +- .../qemuxml2argv-disk-scsi-disk-split.args | 3 +- .../qemuxml2argv-disk-scsi-disk-vpd.args | 3 +- .../qemuxml2argv-disk-scsi-disk-wwn.args | 3 +- .../qemuxml2argv-disk-scsi-lun-passthrough.args | 3 +- .../qemuxml2argv-disk-scsi-megasas.args | 3 +- .../qemuxml2argv-disk-scsi-virtio-scsi.args | 3 +- .../qemuxml2argv-disk-scsi-vscsi.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-serial.args | 3 +- .../qemuxml2argv-disk-snapshot.args | 3 +- .../qemuxml2argv-disk-source-pool-mode.args | 3 +- .../qemuxml2argv-disk-source-pool.args | 3 +- .../qemuxml2argv-disk-usb-device-removable.args | 3 +- .../qemuxml2argv-disk-usb-device.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args | 3 +- .../qemuxml2argv-disk-virtio-ccw-many.args | 3 +- .../qemuxml2argv-disk-virtio-ccw.args | 3 +- .../qemuxml2argv-disk-virtio-s390.args | 3 +- .../qemuxml2argv-disk-virtio-scsi-ccw.args | 3 +- .../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args | 3 +- .../qemuxml2argv-disk-virtio-scsi-ioeventfd.args | 3 +- .../qemuxml2argv-disk-virtio-scsi-max_sectors.args | 3 +- .../qemuxml2argv-disk-virtio-scsi-num_queues.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-virtio.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args | 3 +- .../qemuxml2argv-eoi-disabled.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-event_idx.args | 2 +- .../qemuxml2argv-fips-enabled.args | 3 +- .../qemuxml2argv-floppy-drive-fat.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-fs9p-ccw.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-fs9p.args | 3 +- .../qemuxml2argv-graphics-sdl-fullscreen.args | 3 +- .../qemuxml2argv-graphics-sdl.args | 3 +- ...emuxml2argv-graphics-spice-agent-file-xfer.args | 3 +- .../qemuxml2argv-graphics-spice-agentmouse.args | 3 +- .../qemuxml2argv-graphics-spice-compression.args | 3 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 3 +- .../qemuxml2argv-graphics-spice-sasl.args | 3 +- .../qemuxml2argv-graphics-spice-timeout.args | 3 +- .../qemuxml2argv-graphics-spice-usb-redir.args | 2 +- .../qemuxml2argv-graphics-spice.args | 3 +- .../qemuxml2argv-graphics-vnc-policy.args | 3 +- .../qemuxml2argv-graphics-vnc-sasl.args | 3 +- .../qemuxml2argv-graphics-vnc-socket.args | 3 +- .../qemuxml2argv-graphics-vnc-tls.args | 3 +- .../qemuxml2argv-graphics-vnc-websocket.args | 3 +- .../qemuxml2argv-graphics-vnc.args | 3 +- .../qemuxml2argv-hostdev-pci-address-device.args | 2 +- .../qemuxml2argv-hostdev-pci-address.args | 3 +- .../qemuxml2argv-hostdev-scsi-boot.args | 2 +- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 2 +- .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 2 +- .../qemuxml2argv-hostdev-scsi-lsi.args | 2 +- .../qemuxml2argv-hostdev-scsi-readonly.args | 2 +- ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 2 +- .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args | 2 +- .../qemuxml2argv-hostdev-scsi-virtio-scsi.args | 2 +- ...muxml2argv-hostdev-usb-address-device-boot.args | 3 +- .../qemuxml2argv-hostdev-usb-address-device.args | 3 +- .../qemuxml2argv-hostdev-usb-address.args | 3 +- .../qemuxml2argv-hostdev-vfio-multidomain.args | 2 +- .../qemuxml2argv-hostdev-vfio.args | 2 +- .../qemuxml2argv-hotplug-base.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 2 +- .../qemuxml2argv-hugepages-pages.args | 3 +- .../qemuxml2argv-hugepages-pages2.args | 3 +- .../qemuxml2argv-hugepages-pages3.args | 3 +- .../qemuxml2argv-hugepages-pages5.args | 3 +- .../qemuxml2argv-hugepages-pages6.args | 3 +- .../qemuxml2argv-hugepages-shared.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-hugepages.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-hyperv-off.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 3 +- .../qemuxml2argv-input-usbmouse-addr.args | 3 +- .../qemuxml2argv-input-usbmouse.args | 3 +- .../qemuxml2argv-input-usbtablet.args | 3 +- .../qemuxml2argv-iothreads-disk-virtio-ccw.args | 3 +- .../qemuxml2argv-iothreads-disk.args | 3 +- .../qemuxml2argv-iothreads-ids-partial.args | 3 +- .../qemuxml2argv-iothreads-ids.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 3 +- .../qemuxml2argv-kvm-features-off.args | 2 +- .../qemuxml2argv-kvm-features.args | 3 +- .../qemuxml2argv-kvm-pit-delay.args | 2 +- .../qemuxml2argv-kvm-pit-device.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvm.args | 3 +- .../qemuxml2argv-kvmclock+eoi-disabled.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-lease.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-off-argv.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-off-cap.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-off-caps.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-on-argv.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-on-cap.args | 3 +- .../qemuxml2argv-machine-aeskeywrap-on-caps.args | 3 +- .../qemuxml2argv-machine-aliases1.args | 3 +- .../qemuxml2argv-machine-aliases2.args | 3 +- .../qemuxml2argv-machine-core-off.args | 3 +- .../qemuxml2argv-machine-core-on.args | 3 +- .../qemuxml2argv-machine-deakeywrap-off-argv.args | 3 +- .../qemuxml2argv-machine-deakeywrap-off-cap.args | 3 +- .../qemuxml2argv-machine-deakeywrap-off-caps.args | 3 +- .../qemuxml2argv-machine-deakeywrap-on-argv.args | 3 +- .../qemuxml2argv-machine-deakeywrap-on-cap.args | 3 +- .../qemuxml2argv-machine-deakeywrap-on-caps.args | 3 +- .../qemuxml2argv-machine-keywrap-none-argv.args | 3 +- .../qemuxml2argv-machine-keywrap-none-caps.args | 3 +- .../qemuxml2argv-machine-keywrap-none.args | 3 +- .../qemuxml2argv-machine-usb-opt.args | 3 +- .../qemuxml2argv-machine-vmport-opt.args | 3 +- .../qemuxml2argv-memory-hotplug-dimm-addr.args | 3 +- .../qemuxml2argv-memory-hotplug-dimm.args | 3 +- .../qemuxml2argv-memory-hotplug.args | 3 +- .../qemuxml2argv-memtune-unlimited.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-metadata.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-migrate.args | 3 +- .../qemuxml2argv-minimal-msg-timestamp.args | 2 +- .../qemuxml2argv-minimal-s390.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args | 3 +- .../qemuxml2argv-misc-disable-s3.args | 3 +- .../qemuxml2argv-misc-disable-suspends.args | 3 +- .../qemuxml2argv-misc-enable-s4.args | 3 +- .../qemuxml2argv-misc-no-reboot.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-mlock-off.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-mlock-on.args | 3 +- .../qemuxml2argv-mlock-unsupported.args | 3 +- .../qemuxml2argv-monitor-json.args | 3 +- .../qemuxml2argv-multifunction-pci-device.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-net-client.args | 3 +- .../qemuxml2argv-net-eth-ifname.args | 3 +- .../qemuxml2argv-net-eth-names.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 3 +- .../qemuxml2argv-net-hostdev-multidomain.args | 2 +- .../qemuxml2argv-net-hostdev-vfio-multidomain.args | 2 +- .../qemuxml2argv-net-hostdev-vfio.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-net-hostdev.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-net-server.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 3 +- .../qemuxml2argv-net-vhostuser-multiq.args | 3 +- .../qemuxml2argv-net-vhostuser.args | 3 +- .../qemuxml2argv-net-virtio-ccw.args | 3 +- .../qemuxml2argv-net-virtio-device.args | 3 +- .../qemuxml2argv-net-virtio-disable-offloads.args | 3 +- .../qemuxml2argv-net-virtio-netdev.args | 3 +- .../qemuxml2argv-net-virtio-s390.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-net-virtio.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-no-shutdown.args | 2 +- .../qemuxml2argv-nographics-vga.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-nographics.args | 3 +- .../qemuxml2argv-nosharepages.args | 2 +- ...qemuxml2argv-numad-auto-memory-vcpu-cpuset.args | 3 +- ...d-auto-memory-vcpu-no-cpuset-and-placement.args | 3 +- ...muxml2argv-numad-auto-vcpu-static-numatune.args | 3 +- ...qemuxml2argv-numad-static-memory-auto-vcpu.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-numad.args | 3 +- ...qemuxml2argv-numatune-auto-nodeset-invalid.args | 3 +- .../qemuxml2argv-numatune-auto-prefer.args | 2 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 2 +- .../qemuxml2argv-numatune-memnode.args | 2 +- .../qemuxml2argv-numatune-memory.args | 3 +- .../qemuxml2argv-panic-no-address.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-panic.args | 3 +- .../qemuxml2argv-parallel-parport-chardev.args | 3 +- .../qemuxml2argv-parallel-tcp-chardev.args | 3 +- .../qemuxml2argv-parallel-tcp.args | 3 +- .../qemuxml2argv-pci-autoadd-addr.args | 2 +- .../qemuxml2argv-pci-autoadd-idx.args | 2 +- .../qemuxml2argv-pci-bridge-many-disks.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-many.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args | 2 +- .../qemuxml2argv-pci-serial-dev-chardev.args | 2 +- .../qemuxml2argv-pcie-root-port.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 3 +- .../qemuxml2argv-pcie-switch-downstream-port.args | 3 +- .../qemuxml2argv-pcie-switch-upstream-port.args | 3 +- .../qemuxml2argv-pcihole64-none.args | 3 +- .../qemuxml2argv-pcihole64-q35.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 2 +- .../qemuxml2argv-pmu-feature-off.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-pmu-feature.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 3 +- .../qemuxml2argv-ppce500-serial.args | 2 +- .../qemuxml2argv-pseries-basic.args | 2 +- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- .../qemuxml2argv-pseries-cpu-exact.args | 2 +- .../qemuxml2argv-pseries-cpu-le.args | 2 +- .../qemuxml2argv-pseries-nvram.args | 2 +- .../qemuxml2argv-pseries-panic-missing.args | 2 +- .../qemuxml2argv-pseries-panic-no-address.args | 2 +- .../qemuxml2argv-pseries-usb-default.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-pseries-usb-multi.args | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 2 +- .../qemuxml2argv-pv-spinlock-disabled.args | 3 +- .../qemuxml2argv-pv-spinlock-enabled.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +- .../qemuxml2argv-qemu-ns-no-env.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args | 3 +- .../qemuxml2argv-reboot-timeout-disabled.args | 3 +- .../qemuxml2argv-reboot-timeout-enabled.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-restore-v1.args | 3 +- .../qemuxml2argv-restore-v2-fd.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-restore-v2.args | 3 +- ...muxml2argv-s390-allow-bogus-usb-controller.args | 2 +- .../qemuxml2argv-s390-allow-bogus-usb-none.args | 2 +- .../qemuxml2argv-seclabel-dac-none.args | 3 +- .../qemuxml2argv-seclabel-dynamic-baselabel.args | 3 +- .../qemuxml2argv-seclabel-dynamic-labelskip.args | 3 +- .../qemuxml2argv-seclabel-dynamic-override.args | 3 +- .../qemuxml2argv-seclabel-dynamic-relabel.args | 3 +- .../qemuxml2argv-seclabel-dynamic.args | 3 +- .../qemuxml2argv-seclabel-none.args | 3 +- .../qemuxml2argv-seclabel-static-labelskip.args | 3 +- .../qemuxml2argv-seclabel-static-relabel.args | 2 +- .../qemuxml2argv-seclabel-static.args | 3 +- .../qemuxml2argv-serial-dev-chardev-iobase.args | 3 +- .../qemuxml2argv-serial-dev-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-dev.args | 3 +- .../qemuxml2argv-serial-file-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-file.args | 3 +- .../qemuxml2argv-serial-many-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-many.args | 3 +- .../qemuxml2argv-serial-pty-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-pty.args | 3 +- .../qemuxml2argv-serial-spiceport-nospice.args | 2 +- .../qemuxml2argv-serial-spiceport.args | 2 +- .../qemuxml2argv-serial-tcp-chardev.args | 3 +- .../qemuxml2argv-serial-tcp-telnet-chardev.args | 3 +- .../qemuxml2argv-serial-tcp-telnet.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-tcp.args | 3 +- .../qemuxml2argv-serial-udp-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-udp.args | 3 +- .../qemuxml2argv-serial-unix-chardev.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-serial-unix.args | 3 +- .../qemuxml2argv-serial-vc-chardev.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-serial-vc.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 3 +- .../qemuxml2argv-smartcard-controller.args | 2 +- .../qemuxml2argv-smartcard-host-certificates.args | 2 +- .../qemuxml2argv-smartcard-host.args | 2 +- ...emuxml2argv-smartcard-passthrough-spicevmc.args | 2 +- .../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-smp.args | 3 +- .../qemuxml2argv-sound-device.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-sound.args | 3 +- .../qemuxml2argv-tpm-passthrough.args | 3 +- .../qemuxml2argv-usb-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args | 2 +- .../qemuxml2argv-usb-ich9-companion.args | 2 +- .../qemuxml2argv-usb-ich9-ehci-addr.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-none.args | 2 +- .../qemuxml2argv-usb-piix3-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args | 2 +- .../qemuxml2argv-usb-redir-boot.args | 2 +- .../qemuxml2argv-usb-redir-filter-version.args | 2 +- .../qemuxml2argv-usb-redir-filter.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 3 +- .../qemuxml2argv-video-qxl-device-vgamem.args | 3 +- .../qemuxml2argv-video-qxl-device.args | 3 +- .../qemuxml2argv-video-qxl-nodevice.args | 3 +- .../qemuxml2argv-video-qxl-sec-device-vgamem.args | 3 +- .../qemuxml2argv-video-qxl-sec-device.args | 3 +- .../qemuxml2argv-video-vga-device-vgamem.args | 3 +- .../qemuxml2argv-video-vga-device.args | 3 +- .../qemuxml2argv-video-vga-nodevice.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-virtio-lun.args | 2 +- .../qemuxml2argv-virtio-rng-ccw.args | 2 +- .../qemuxml2argv-virtio-rng-default.args | 3 +- .../qemuxml2argv-virtio-rng-egd.args | 3 +- .../qemuxml2argv-virtio-rng-multiple.args | 3 +- .../qemuxml2argv-virtio-rng-random.args | 3 +- .../qemuxml2argv-watchdog-device.args | 3 +- .../qemuxml2argv-watchdog-diag288.args | 2 +- .../qemuxml2argv-watchdog-dump.args | 3 +- .../qemuxml2argv-watchdog-injectnmi.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-watchdog.args | 3 +- tests/qemuxml2argvtest.c | 19 +++++--- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++--- 457 files changed, 952 insertions(+), 474 deletions(-) -- 2.5.0
In virSecuritySELinuxSetSecurityChardevLabel() we are labelling unix socket path, but accessing another structure of the union. This does not pose a problem currently as both paths are at the same offset, but this should be fixed for the future. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 6e67a86cc27c..a37f8c140b07 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1740,7 +1740,7 @@ virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def, case VIR_DOMAIN_CHR_TYPE_UNIX: if (!dev_source->data.nix.listen) { - if (virSecuritySELinuxSetFilecon(dev_source->data.file.path, + if (virSecuritySELinuxSetFilecon(dev_source->data.nix.path, imagelabel) < 0) goto done; } -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:42PM -0700, Martin Kletzander wrote:
In virSecuritySELinuxSetSecurityChardevLabel() we are labelling unix socket path, but accessing another structure of the union. This does not pose a problem currently as both paths are at the same offset, but this should be fixed for the future.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
SELinux security driver already does that, but DAC driver somehow missed the memo. Let's fix it so it works the same way. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_dac.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index deb69803095b..bed23c30ae8c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -789,6 +789,15 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, ret = 0; break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (!dev_source->data.nix.listen) { + if (virSecurityDACSetOwnership(dev_source->data.nix.path, + user, group) < 0) + goto done; + } + ret = 0; + break; + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: @@ -796,7 +805,6 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_UDP: case VIR_DOMAIN_CHR_TYPE_TCP: - case VIR_DOMAIN_CHR_TYPE_UNIX: case VIR_DOMAIN_CHR_TYPE_SPICEVMC: case VIR_DOMAIN_CHR_TYPE_NMDM: case VIR_DOMAIN_CHR_TYPE_LAST: -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:43PM -0700, Martin Kletzander wrote:
SELinux security driver already does that, but DAC driver somehow missed the memo. Let's fix it so it works the same way.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_dac.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
That function can be used for setting security labels on arbitrary directories. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_driver.h | 5 +++++ src/security/security_manager.c | 17 +++++++++++++++++ src/security/security_manager.h | 4 ++++ 4 files changed, 27 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eb42bfa519e2..f09acf286a28 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1026,6 +1026,7 @@ virSecurityDriverLookup; # security/security_manager.h virSecurityManagerCheckAllLabel; virSecurityManagerClearSocketLabel; +virSecurityManagerDomainSetDirLabel; virSecurityManagerGenLabel; virSecurityManagerGetBaseLabel; virSecurityManagerGetDOI; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index f0dca09177e5..784b0dee65ea 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -118,6 +118,9 @@ typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src); +typedef int (*virSecurityDomainSetDirLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path); struct _virSecurityDriver { @@ -168,6 +171,8 @@ struct _virSecurityDriver { virSecurityDomainSetHugepages domainSetSecurityHugepages; virSecurityDriverGetBaseLabel getBaseLabel; + + virSecurityDomainSetDirLabel domainSetDirLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index b0cd9e856903..1098558b0755 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -991,3 +991,20 @@ virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, return 0; } + + +int +virSecurityManagerDomainSetDirLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + if (mgr->drv->domainSetDirLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetDirLabel(mgr, vm, path); + virObjectUnlock(mgr); + return ret; + } + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 13468db3997b..78f34a033fdd 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -150,4 +150,8 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virStorageSourcePtr src); +int virSecurityManagerDomainSetDirLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path); + #endif /* VIR_SECURITY_MANAGER_H__ */ -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:44PM -0700, Martin Kletzander wrote:
That function can be used for setting security labels on arbitrary directories.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_driver.h | 5 +++++ src/security/security_manager.c | 17 +++++++++++++++++ src/security/security_manager.h | 4 ++++ 4 files changed, 27 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_stack.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 1ded57b9604c..8d9560dad545 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -599,6 +599,24 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, return rc; } +static int +virSecurityStackDomainSetDirLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + const char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerDomainSetDirLabel(item->securityManager, + vm, path) < 0) + rc = -1; + } + + return rc; +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -648,4 +666,6 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityHugepages = virSecurityStackSetHugepages, .getBaseLabel = virSecurityStackGetBaseLabel, + + .domainSetDirLabel = virSecurityStackDomainSetDirLabel, }; -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:45PM -0700, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_stack.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_selinux.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a37f8c140b07..d4bfee76c5a5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2505,6 +2505,21 @@ virSecuritySELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, return opts; } +static char * +virSecuritySELinuxDomainSetDirLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + virSecurityLabelDefPtr seclabel; + int ret = -1; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!seclabel || !seclabel->relabel) + return 0; + + return virSecuritySELinuxSetFilecon(path, seclabel->imagelabel); +} + virSecurityDriver virSecurityDriverSELinux = { .privateDataLen = sizeof(virSecuritySELinuxData), .name = SECURITY_SELINUX_NAME, @@ -2549,4 +2564,6 @@ virSecurityDriver virSecurityDriverSELinux = { .domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions, .getBaseLabel = virSecuritySELinuxGetBaseLabel, + + .domainSetDirLabel = virSecuritySELinuxDomainSetDirLabel, }; -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:46PM -0700, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_selinux.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_dac.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bed23c30ae8c..864d75b2fd9c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1394,6 +1394,24 @@ virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr, return priv->baselabel; } +static int +virSecurityDACDomainSetDirLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + uid_t user; + gid_t group; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) + return -1; + + return virSecurityDACSetOwnership(path, user, group); +} + virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), .name = SECURITY_DAC_NAME, @@ -1441,4 +1459,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, .getBaseLabel = virSecurityDACGetBaseLabel, + + .domainSetDirLabel = virSecurityDACDomainSetDirLabel, }; -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:47PM -0700, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/security/security_dac.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
We are automatically generating some socket paths for domains, but all those paths end up in a directory that's the same for multiple domains. The problem is that multiple domains can each run with different seclabels (users, selinux contexts, etc.). The idea here is to create a per-domain directory labelled in a way that each domain can access its own unix sockets. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 +++--- src/qemu/qemu_process.c | 57 +++++++++++++++++++++- .../qemuxml2argv-channel-virtio-unix.args | 7 +-- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5a4fdf09a2b..abc57d762075 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8051,7 +8051,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket && virAsprintf(&graphics->data.vnc.socket, - "%s/%s.vnc", cfg->libDir, def->name) == -1) + "%s/domain-%s/vnc.sock", cfg->libDir, def->name) == -1) goto error; virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 84e5fa530cba..0a9ed6babb4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1307,16 +1307,12 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto cleanup; } - if (dev->data.chr->target.name) { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s", - cfg->channelTargetDir, - def->name, dev->data.chr->target.name) < 0) - goto cleanup; - } else { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s", - cfg->channelTargetDir, def->name) < 0) - goto cleanup; - } + if (virAsprintf(&dev->data.chr->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name, + dev->data.chr->target.name ? dev->data.chr->target.name + : "unknown.sock") < 0) + goto cleanup; dev->data.chr->source.data.nix.listen = true; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 505778ec2f05..93ccd2a706b9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -46,6 +46,7 @@ #include "cpu/cpu.h" #include "datatypes.h" +#include "dirname.h" #include "virlog.h" #include "virerror.h" #include "viralloc.h" @@ -3265,7 +3266,7 @@ qemuProcessPrepareMonitorChr(virQEMUDriverConfigPtr cfg, monConfig->type = VIR_DOMAIN_CHR_TYPE_UNIX; monConfig->data.nix.listen = true; - if (virAsprintf(&monConfig->data.nix.path, "%s/%s.monitor", + if (virAsprintf(&monConfig->data.nix.path, "%s/domain-%s/monitor.sock", cfg->libDir, vm) < 0) return -1; return 0; @@ -4389,6 +4390,7 @@ int qemuProcessStart(virConnectPtr conn, unsigned int hostdev_flags = 0; size_t nnicindexes = 0; int *nicindexes = NULL; + char *tmppath = NULL, *tmpdirpath = NULL; VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s stdin_fd=%d " "stdin_path=%s snapshot=%p vmop=%d flags=0x%x", @@ -4725,6 +4727,44 @@ int qemuProcessStart(virConnectPtr conn, &nnicindexes, &nicindexes))) goto cleanup; + + /* + * Create all per-domain directories in order to make sure domain + * with any possible seclabels can access it. + */ + if (virAsprintf(&tmppath, "%s/domain-%s", cfg->libDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(tmppath) < 0) + goto cleanup; + + if (!(tmpdirpath = mdir_name(tmppath))) + goto cleanup; + + if (virSecurityManagerDomainSetDirLabel(driver->securityManager, + vm->def, tmpdirpath) < 0) + goto cleanup; + + VIR_FREE(tmppath); + VIR_FREE(tmpdirpath); + + if (virAsprintf(&tmppath, "%s/domain-%s", + cfg->channelTargetDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(tmppath) < 0) + goto cleanup; + + if (!(tmpdirpath = mdir_name(tmppath))) + goto cleanup; + + if (virSecurityManagerDomainSetDirLabel(driver->securityManager, + vm->def, tmpdirpath) < 0) + goto cleanup; + + VIR_FREE(tmpdirpath); + VIR_FREE(tmppath); + /* now that we know it is about to start call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -5078,6 +5118,8 @@ int qemuProcessStart(virConnectPtr conn, /* We jump here if we failed to start the VM for any reason, or * if we failed to initialize the now running VM. kill it off and * pretend we never started it */ + VIR_FREE(tmppath); + VIR_FREE(tmpdirpath); VIR_FREE(nodeset); virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); @@ -5140,6 +5182,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, size_t i; int logfile = -1; char *timestamp; + char *tmppath = NULL; char ebuf[1024]; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -5230,6 +5273,18 @@ void qemuProcessStop(virQEMUDriverPtr driver, priv->monConfig = NULL; } + ignore_value(virAsprintf(&tmppath, "%s/domain-%s", + cfg->libDir, vm->def->name)); + if (tmppath) + virFileDeleteTree(tmppath); + VIR_FREE(tmppath); + + ignore_value(virAsprintf(&tmppath, "%s/domain-%s", + cfg->channelTargetDir, vm->def->name)); + if (tmppath) + virFileDeleteTree(tmppath); + VIR_FREE(tmppath); + ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args index 43a34ced80ed..289b9d700b15 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args @@ -9,11 +9,12 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -usb \ -hda /dev/HostVG/QEMUGuest1 \ -chardev socket,id=charchannel0,path=\ -/tmp/QEMUGuest1.org.qemu.guest_agent.0,server,nowait \ +/tmp/domain-QEMUGuest1/org.qemu.guest_agent.0,server,nowait \ -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,\ name=org.qemu.guest_agent.0 \ --chardev socket,id=charchannel1,path=/tmp/QEMUGuest1,server,nowait \ +-chardev \ +socket,id=charchannel1,path=/tmp/domain-QEMUGuest1/unknown.sock,server,nowait \ -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1 \ --chardev socket,id=charchannel2,path=/tmp/QEMUGuest1.ble,server,nowait \ +-chardev socket,id=charchannel2,path=/tmp/domain-QEMUGuest1/ble,server,nowait \ -device virtserialport,bus=virtio-serial0.0,nr=3,chardev=charchannel2,id=channel2,\ name=ble -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:48PM -0700, Martin Kletzander wrote:
We are automatically generating some socket paths for domains, but all those paths end up in a directory that's the same for multiple domains. The problem is that multiple domains can each run with different seclabels (users, selinux contexts, etc.). The idea here is to create a per-domain directory labelled in a way that each domain can access its own unix sockets.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 +++--- src/qemu/qemu_process.c | 57 +++++++++++++++++++++- .../qemuxml2argv-channel-virtio-unix.args | 7 +-- 4 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5a4fdf09a2b..abc57d762075 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8051,7 +8051,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket && virAsprintf(&graphics->data.vnc.socket, - "%s/%s.vnc", cfg->libDir, def->name) == -1) + "%s/domain-%s/vnc.sock", cfg->libDir, def->name) == -1) goto error;
virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 84e5fa530cba..0a9ed6babb4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1307,16 +1307,12 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto cleanup; }
- if (dev->data.chr->target.name) { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s", - cfg->channelTargetDir, - def->name, dev->data.chr->target.name) < 0) - goto cleanup; - } else { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s", - cfg->channelTargetDir, def->name) < 0) - goto cleanup; - } + if (virAsprintf(&dev->data.chr->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name, + dev->data.chr->target.name ? dev->data.chr->target.name + : "unknown.sock") < 0) + goto cleanup;
This worries me a little - IIUC we could end up with multiple devices using the same "unknown.sock" file path. If I'm reading correctly, the original code had this problem too. Would we be justified in raising an error in this scenario ? ACK anyway, since it appears the problem already existed. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Aug 24, 2015 at 10:32:03AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 17, 2015 at 12:16:48PM -0700, Martin Kletzander wrote:
We are automatically generating some socket paths for domains, but all those paths end up in a directory that's the same for multiple domains. The problem is that multiple domains can each run with different seclabels (users, selinux contexts, etc.). The idea here is to create a per-domain directory labelled in a way that each domain can access its own unix sockets.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 +++--- src/qemu/qemu_process.c | 57 +++++++++++++++++++++- .../qemuxml2argv-channel-virtio-unix.args | 7 +-- 4 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5a4fdf09a2b..abc57d762075 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8051,7 +8051,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket && virAsprintf(&graphics->data.vnc.socket, - "%s/%s.vnc", cfg->libDir, def->name) == -1) + "%s/domain-%s/vnc.sock", cfg->libDir, def->name) == -1) goto error;
virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 84e5fa530cba..0a9ed6babb4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1307,16 +1307,12 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto cleanup; }
- if (dev->data.chr->target.name) { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s", - cfg->channelTargetDir, - def->name, dev->data.chr->target.name) < 0) - goto cleanup; - } else { - if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s", - cfg->channelTargetDir, def->name) < 0) - goto cleanup; - } + if (virAsprintf(&dev->data.chr->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name, + dev->data.chr->target.name ? dev->data.chr->target.name + : "unknown.sock") < 0) + goto cleanup;
This worries me a little - IIUC we could end up with multiple devices using the same "unknown.sock" file path. If I'm reading correctly, the original code had this problem too. Would we be justified in raising an error in this scenario ?
ACK anyway, since it appears the problem already existed.
To be honest, this worried me a *lot* actually. But since it already existed and I spent way too much trying to fix this as cleanly as possible, I rather posted it as is thinking the porblem might be dealt with later on, although I'm not quite sure it's worth the code change. Thanks for the review.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
The output of that function was not tested until now. In order to keep the paths in /tmp, the test driver config is "fixed" as well. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++++++++++------ 8 files changed, 20 insertions(+), 13 deletions(-) diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns0.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns0.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns0.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns0.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns1.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns1.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns1.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline-ns1.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline-ns0.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline-ns0.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline-ns0.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline-ns0.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-commandline.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args index ad16f43ed00e..dac865140aec 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none ARGUMENT diff --git a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args index 26038a032793..ce2bf40978dc 100644 --- a/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args +++ b/tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +unix:/tmp/domain-QEMUGuest1/monitor.sock,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 8eaab8a5a871..a68e7620153c 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -16,6 +16,7 @@ # include "qemu/qemu_capabilities.h" # include "qemu/qemu_command.h" # include "qemu/qemu_domain.h" +# include "qemu/qemu_process.h" # include "datatypes.h" # include "cpu/cpu_map.h" # include "testutilsqemu.h" @@ -37,7 +38,7 @@ static int testCompareXMLToArgvFiles(const char *xml, char *actualargv = NULL; int ret = -1; virDomainDefPtr vmdef = NULL; - virDomainChrSourceDef monitor_chr; + virDomainChrSourceDefPtr monitor_chr = NULL; virConnectPtr conn; char *log = NULL; char *emulator = NULL; @@ -77,10 +78,12 @@ static int testCompareXMLToArgvFiles(const char *xml, vmdef->id = -1; - memset(&monitor_chr, 0, sizeof(monitor_chr)); - monitor_chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; - monitor_chr.data.nix.path = (char *)"/tmp/test-monitor"; - monitor_chr.data.nix.listen = true; + if (VIR_ALLOC(monitor_chr) < 0) + goto fail; + if (qemuProcessPrepareMonitorChr(driver.config, + monitor_chr, + vmdef->name) < 0) + goto fail; virQEMUCapsSetList(extraFlags, QEMU_CAPS_VNC_COLON, @@ -104,7 +107,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; if (!(cmd = qemuBuildCommandLine(conn, &driver, - vmdef, &monitor_chr, json, extraFlags, + vmdef, monitor_chr, json, extraFlags, migrateFrom, migrateFd, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &testCallbacks, false, false, NULL, @@ -142,6 +145,7 @@ static int testCompareXMLToArgvFiles(const char *xml, ret = 0; fail: + virDomainChrSourceDefFree(monitor_chr); VIR_FREE(log); VIR_FREE(emulator); VIR_FREE(actualargv); @@ -199,6 +203,9 @@ mymain(void) if (!(driver.config = virQEMUDriverConfigNew(false))) return EXIT_FAILURE; + VIR_FREE(driver.config->libDir); + if (VIR_STRDUP_QUIET(driver.config->libDir, "/tmp") < 0) + return EXIT_FAILURE; if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) -- 2.5.0
On Mon, Aug 17, 2015 at 12:16:49PM -0700, Martin Kletzander wrote:
The output of that function was not tested until now. In order to keep the paths in /tmp, the test driver config is "fixed" as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++++++++++------ 8 files changed, 20 insertions(+), 13 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Aug 24, 2015 at 10:32:43AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 17, 2015 at 12:16:49PM -0700, Martin Kletzander wrote:
The output of that function was not tested until now. In order to keep the paths in /tmp, the test driver config is "fixed" as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++++++++++------ 8 files changed, 20 insertions(+), 13 deletions(-)
ACK
Thanks for the review, should I keep it as a separate patch or squash it into the previous one (I prefer the former)? How about the qemuxml2argvtest, do we want to modify that as well if the function is already being tested here? I'm referring to the original question in the cover letter. Martin
On Mon, Aug 24, 2015 at 11:50:13AM +0200, Martin Kletzander wrote:
On Mon, Aug 24, 2015 at 10:32:43AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 17, 2015 at 12:16:49PM -0700, Martin Kletzander wrote:
The output of that function was not tested until now. In order to keep the paths in /tmp, the test driver config is "fixed" as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++++++++++------ 8 files changed, 20 insertions(+), 13 deletions(-)
ACK
Thanks for the review, should I keep it as a separate patch or squash it into the previous one (I prefer the former)? How about the qemuxml2argvtest, do we want to modify that as well if the function is already being tested here? I'm referring to the original question in the cover letter.
Squash it if it is needed to ensure git bisect succeeds. I don't really mind either way about the qemuxml2argvtest - it is sufficient to have the codepath tested by qemuxmlnstest IMHO. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Aug 24, 2015 at 10:58:08AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 24, 2015 at 11:50:13AM +0200, Martin Kletzander wrote:
On Mon, Aug 24, 2015 at 10:32:43AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 17, 2015 at 12:16:49PM -0700, Martin Kletzander wrote:
The output of that function was not tested until now. In order to keep the paths in /tmp, the test driver config is "fixed" as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- .../qemuxmlns-qemu-ns-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-commandline-ns1.args | 2 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-commandline.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.args | 2 +- .../qemuxmlns-qemu-ns-domain-commandline.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.args | 2 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.args | 2 +- tests/qemuxmlnstest.c | 19 +++++++++++++------ 8 files changed, 20 insertions(+), 13 deletions(-)
ACK
Thanks for the review, should I keep it as a separate patch or squash it into the previous one (I prefer the former)? How about the qemuxml2argvtest, do we want to modify that as well if the function is already being tested here? I'm referring to the original question in the cover letter.
Squash it if it is needed to ensure git bisect succeeds.
It is not, the build will work after every commit, so I'll keep it separated.
I don't really mind either way about the qemuxml2argvtest - it is sufficient to have the codepath tested by qemuxmlnstest IMHO.
Agreed, I'll drop that excessively and necessarily long patch before pushing the series. Once again thanks for the review. Martin
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Martin Kletzander