src/security/virt-aa-helper.c | 35 +++++++++++++++++++++++++++++++++++ tests/virt-aa-helper-test | 15 +++++++++++++++ 2 files changed, 50 insertions(+)
Cole was recently adding a few of the usual apparmor suspects to BZ 1761645 and I was taking a look at the low hanging fruits of it today. It isn't perfect, but would resolve the reported issue - so I'd appreciate a review. Limitations: - One could break the path creating elements in qemuBuildShmemBackendMemProps and qemuDomainPrepareShmemChardev into extra functions and then use those from virt-aa-helper. But I haven't done so yet and unless it is strictly required consider it too much for what we want/need to achieve here. - I haven't covered hotplug of shmem devices yet, it seems there is no infrastructure for their labels yet and I wasn't sure how important shmem-hotplug would even be to anyone. Christian Ehrhardt (2): virt-aa-helper: add rules for shmem devices virt-aa-helper: testcase for shmem devices src/security/virt-aa-helper.c | 35 +++++++++++++++++++++++++++++++++++ tests/virt-aa-helper-test | 15 +++++++++++++++ 2 files changed, 50 insertions(+) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Cole was recently adding a few of the usual apparmor suspects to BZ 1761645 and I was taking a look at the low hanging fruits of it today. It isn't perfect, but would resolve the reported issue - so I'd appreciate a review. Limitations: - One could break the path creating elements in qemuBuildShmemBackendMemProps and qemuDomainPrepareShmemChardev into extra functions and then use those from virt-aa-helper. But I haven't done so yet and unless it is strictly required consider it too much for what we want/need to achieve here. - I haven't covered hotplug of shmem devices yet, it seems there is no infrastructure for their labels yet and I wasn't sure how important shmem-hotplug would even be to anyone. Updates in v2: - rebase latest master - drop checking shmems[i] - switch to use g_strdup_printf - added an extra patch removing checks like ctl->def->mems[i] - add reviewed-by tag Christian Ehrhardt (3): virt-aa-helper: add rules for shmem devices virt-aa-helper: testcase for shmem devices virt-aa-helper: drop pointer checks in get_files src/security/virt-aa-helper.c | 166 +++++++++++++++++++--------------- tests/virt-aa-helper-test | 15 +++ 2 files changed, 109 insertions(+), 72 deletions(-) -- 2.24.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 14, 2019 at 12:20 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote: > > Cole was recently adding a few of the usual apparmor suspects to BZ 1761645 > and I was taking a look at the low hanging fruits of it today. It isn't > perfect, but would resolve the reported issue - so I'd appreciate a > review. > > Limitations: > - One could break the path creating elements in qemuBuildShmemBackendMemProps > and qemuDomainPrepareShmemChardev into extra functions and then use those > from virt-aa-helper. But I haven't done so yet and unless it is strictly > required consider it too much for what we want/need to achieve here. > - I haven't covered hotplug of shmem devices yet, it seems there is no > infrastructure for their labels yet and I wasn't sure how important > shmem-hotplug would even be to anyone. > > Updates in v2: > - rebase latest master > - drop checking shmems[i] > - switch to use g_strdup_printf > - added an extra patch removing checks like ctl->def->mems[i] > - add reviewed-by tag Thanks everyone for the further review and discussion. Pushing with Ack/Review Tags after a rebuild/recheck > Christian Ehrhardt (3): > virt-aa-helper: add rules for shmem devices > virt-aa-helper: testcase for shmem devices > virt-aa-helper: drop pointer checks in get_files > > src/security/virt-aa-helper.c | 166 +++++++++++++++++++--------------- > tests/virt-aa-helper-test | 15 +++ > 2 files changed, 109 insertions(+), 72 deletions(-) > > -- > 2.24.0 > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Shared memory devices need qemu to be able to access certain paths
either for the shared memory directly (mostly ivshmem-plain) or for a
socket (mostly ivshmem-doorbell).
Add logic to virt-aa-helper to render those apparmor rules based
on the domain configuration.
https://bugzilla.redhat.com/show_bug.cgi?id=1761645
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
src/security/virt-aa-helper.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5ac9a9eeb8..c6c4bb9bd0 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -929,6 +929,7 @@ get_files(vahControl * ctl)
int rc = -1;
size_t i;
char *uuid;
+ char *mem_path = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
bool needsVfio = false, needsvhost = false, needsgl = false;
@@ -1192,6 +1193,35 @@ get_files(vahControl * ctl)
}
}
+ for (i = 0; i < ctl->def->nshmems; i++) {
+ virDomainShmemDef *shmem = ctl->def->shmems[i];
+ /* server path can be on any type and overwrites defaults */
+ if (shmem->server.enabled &&
+ shmem->server.chr.data.nix.path) {
+ if (vah_add_file(&buf, shmem->server.chr.data.nix.path,
+ "rw") != 0)
+ goto cleanup;
+ } else {
+ switch (shmem->model) {
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+ /* until exposed, recreate qemuBuildShmemBackendMemProps */
+ mem_path = g_strdup_printf("/dev/shm/%s", shmem->name);
+ break;
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
+ /* until exposed, recreate qemuDomainPrepareShmemChardev */
+ mem_path = g_strdup_printf("/var/lib/libvirt/shmem-%s-sock",
+ shmem->name);
+ break;
+ }
+ if (mem_path != NULL) {
+ if (vah_add_file(&buf, mem_path, "rw") != 0)
+ goto cleanup;
+ }
+ }
+ }
+
+
if (ctl->def->tpm) {
char *shortName = NULL;
const char *tpmpath = NULL;
@@ -1286,6 +1316,7 @@ get_files(vahControl * ctl)
ctl->files = virBufferContentAndReset(&buf);
cleanup:
+ VIR_FREE(mem_path);
VIR_FREE(uuid);
return rc;
}
--
2.24.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Adding build time self tests for basic (deprecated), doorbell and plain mode.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
tests/virt-aa-helper-test | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 6e674bfe5c..6a6703ecf5 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -384,6 +384,21 @@ testme "0" "dri egl" "-r -u $valid_uuid" "$test_xml" "/dev/dri/testegl1.*rw,$"
sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<graphics type='spice'><gl enable='yes' rendernode='/dev/dri/testegl2'/></graphics></devices>,g" "$template_xml" > "$test_xml"
testme "0" "dri spice" "-r -u $valid_uuid" "$test_xml" "/dev/dri/testegl2.*rw,$"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<shmem name='my_shmem0'><model type='ivshmem'/><size unit='M'>4</size></shmem></devices>,g" "$template_xml" > "$test_xml"
+testme "0" "shmem" "-r -u $valid_uuid" "$test_xml" "\"/var/lib/libvirt/shmem-my_shmem0-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<shmem name='my_shmem0'><model type='ivshmem'/><size unit='M'>4</size><server path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g" "$template_xml" > "$test_xml"
+testme "0" "shmem serverpath" "-r -u $valid_uuid" "$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<shmem name='my_shmem0'><model type='ivshmem-plain'/><size unit='M'>4</size></shmem></devices>,g" "$template_xml" > "$test_xml"
+testme "0" "shmem plain" "-r -u $valid_uuid" "$test_xml" "\"/dev/shm/my_shmem0\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<shmem name='shmem_server'><model type='ivshmem-doorbell'/></shmem></devices>,g" "$template_xml" > "$test_xml"
+testme "0" "shmem doorbell" "-r -u $valid_uuid" "$test_xml" "\"/var/lib/libvirt/shmem-shmem_server-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<shmem name='shmem_server'><model type='ivshmem-doorbell'/><server path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g" "$template_xml" > "$test_xml"
+testme "0" "shmem doorbell serverpath" "-r -u $valid_uuid" "$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
testme "0" "help" "-h"
echo "" >$output
--
2.24.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
It was mentioned that the pointers in loops like:
for (i = 0; i < ctl->def->nserials; i++)
if (ctl->def->serials[i] ...
will always be !NULL or we would have a much more serious problem.
Simplify the if chains in get_files by dropping these checks.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------
1 file changed, 63 insertions(+), 72 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c6c4bb9bd0..17f49a6259 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -965,8 +965,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nserials; i++)
- if (ctl->def->serials[i] &&
- (ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
+ if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV ||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE ||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX ||
@@ -980,8 +979,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nconsoles; i++)
- if (ctl->def->consoles[i] &&
- (ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
+ if ((ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV ||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE ||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX ||
@@ -993,8 +991,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nparallels; i++)
- if (ctl->def->parallels[i] &&
- (ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
+ if ((ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV ||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE ||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX ||
@@ -1008,8 +1005,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nchannels; i++)
- if (ctl->def->channels[i] &&
- (ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
+ if ((ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY ||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV ||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE ||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX ||
@@ -1082,76 +1078,74 @@ get_files(vahControl * ctl)
"r") != 0)
goto cleanup;
- for (i = 0; i < ctl->def->nhostdevs; i++)
- if (ctl->def->hostdevs[i]) {
- virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
- virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
- switch (dev->source.subsys.type) {
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- virUSBDevicePtr usb =
- virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
+ for (i = 0; i < ctl->def->nhostdevs; i++) {
+ virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
+ virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
+ switch (dev->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
+ virUSBDevicePtr usb =
+ virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
- if (usb == NULL)
- continue;
-
- if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
- continue;
+ if (usb == NULL)
+ continue;
- rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
- virUSBDeviceFree(usb);
- if (rc != 0)
- goto cleanup;
- break;
- }
+ if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
+ continue;
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
- virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
- switch ((virMediatedDeviceModelType) mdevsrc->model) {
- case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
- case VIR_MDEV_MODEL_TYPE_VFIO_AP:
- case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
- needsVfio = true;
- break;
- case VIR_MDEV_MODEL_TYPE_LAST:
- default:
- virReportEnumRangeError(virMediatedDeviceModelType,
- mdevsrc->model);
- break;
- }
- break;
- }
-
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- virPCIDevicePtr pci = virPCIDeviceNew(
- dev->source.subsys.u.pci.addr.domain,
- dev->source.subsys.u.pci.addr.bus,
- dev->source.subsys.u.pci.addr.slot,
- dev->source.subsys.u.pci.addr.function);
+ rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
+ virUSBDeviceFree(usb);
+ if (rc != 0)
+ goto cleanup;
+ break;
+ }
- virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend;
- if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
- backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+ virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev;
+ switch ((virMediatedDeviceModelType) mdevsrc->model) {
+ case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+ case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+ case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
needsVfio = true;
- }
+ break;
+ case VIR_MDEV_MODEL_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virMediatedDeviceModelType,
+ mdevsrc->model);
+ break;
+ }
+ break;
+ }
- if (pci == NULL)
- continue;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
+ virPCIDevicePtr pci = virPCIDeviceNew(
+ dev->source.subsys.u.pci.addr.domain,
+ dev->source.subsys.u.pci.addr.bus,
+ dev->source.subsys.u.pci.addr.slot,
+ dev->source.subsys.u.pci.addr.function);
+
+ virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend;
+ if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
+ backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ needsVfio = true;
+ }
- rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
- virPCIDeviceFree(pci);
+ if (pci == NULL)
+ continue;
- break;
- }
+ rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
+ virPCIDeviceFree(pci);
- default:
- rc = 0;
- break;
- } /* switch */
+ break;
}
+ default:
+ rc = 0;
+ break;
+ } /* switch */
+ }
+
for (i = 0; i < ctl->def->nfss; i++) {
- if (ctl->def->fss[i] &&
- ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
+ if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
(ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) &&
ctl->def->fss[i]->src) {
@@ -1166,16 +1160,14 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->ninputs; i++) {
- if (ctl->def->inputs[i] &&
- ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
+ if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev, "rw") != 0)
goto cleanup;
}
}
for (i = 0; i < ctl->def->nnets; i++) {
- if (ctl->def->nets[i] &&
- ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
+ if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
ctl->def->nets[i]->data.vhostuser) {
virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;
@@ -1186,8 +1178,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nmems; i++) {
- if (ctl->def->mems[i] &&
- ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0)
goto cleanup;
}
--
2.24.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/14/19 6:20 AM, Christian Ehrhardt wrote: > It was mentioned that the pointers in loops like: > for (i = 0; i < ctl->def->nserials; i++) > if (ctl->def->serials[i] ... > will always be !NULL or we would have a much more serious problem. > > Simplify the if chains in get_files by dropping these checks. > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------ > 1 file changed, 63 insertions(+), 72 deletions(-) > Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > It was mentioned that the pointers in loops like: > for (i = 0; i < ctl->def->nserials; i++) > if (ctl->def->serials[i] ... > will always be !NULL or we would have a much more serious problem. > > Simplify the if chains in get_files by dropping these checks. I don't really see why a NULL check is a problem so this feels a bit like busy work and it seems short-circuiting and not doing is exactly what you would want to do in the event of a 'much more serious problem'. Is this really required? +0 to apply on principle (I've not reviewed the changes closely). > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------ > 1 file changed, 63 insertions(+), 72 deletions(-) > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index c6c4bb9bd0..17f49a6259 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -965,8 +965,7 @@ get_files(vahControl * ctl) > } > > for (i = 0; i < ctl->def->nserials; i++) > - if (ctl->def->serials[i] && > - (ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > + if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > @@ -980,8 +979,7 @@ get_files(vahControl * ctl) > goto cleanup; > > for (i = 0; i < ctl->def->nconsoles; i++) > - if (ctl->def->consoles[i] && > - (ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > + if ((ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > @@ -993,8 +991,7 @@ get_files(vahControl * ctl) > goto cleanup; > > for (i = 0; i < ctl->def->nparallels; i++) > - if (ctl->def->parallels[i] && > - (ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > + if ((ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > @@ -1008,8 +1005,7 @@ get_files(vahControl * ctl) > goto cleanup; > > for (i = 0; i < ctl->def->nchannels; i++) > - if (ctl->def->channels[i] && > - (ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > + if ((ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > @@ -1082,76 +1078,74 @@ get_files(vahControl * ctl) > "r") != 0) > goto cleanup; > > - for (i = 0; i < ctl->def->nhostdevs; i++) > - if (ctl->def->hostdevs[i]) { > - virDomainHostdevDefPtr dev = ctl->def->hostdevs[i]; > - virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; > - switch (dev->source.subsys.type) { > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { > - virUSBDevicePtr usb = > - virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL); > + for (i = 0; i < ctl->def->nhostdevs; i++) { > + virDomainHostdevDefPtr dev = ctl->def->hostdevs[i]; > + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; > + switch (dev->source.subsys.type) { > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { > + virUSBDevicePtr usb = > + virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL); > > - if (usb == NULL) > - continue; > - > - if (virHostdevFindUSBDevice(dev, true, &usb) < 0) > - continue; > + if (usb == NULL) > + continue; > > - rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf); > - virUSBDeviceFree(usb); > - if (rc != 0) > - goto cleanup; > - break; > - } > + if (virHostdevFindUSBDevice(dev, true, &usb) < 0) > + continue; > > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { > - virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev; > - switch ((virMediatedDeviceModelType) mdevsrc->model) { > - case VIR_MDEV_MODEL_TYPE_VFIO_PCI: > - case VIR_MDEV_MODEL_TYPE_VFIO_AP: > - case VIR_MDEV_MODEL_TYPE_VFIO_CCW: > - needsVfio = true; > - break; > - case VIR_MDEV_MODEL_TYPE_LAST: > - default: > - virReportEnumRangeError(virMediatedDeviceModelType, > - mdevsrc->model); > - break; > - } > - break; > - } > - > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { > - virPCIDevicePtr pci = virPCIDeviceNew( > - dev->source.subsys.u.pci.addr.domain, > - dev->source.subsys.u.pci.addr.bus, > - dev->source.subsys.u.pci.addr.slot, > - dev->source.subsys.u.pci.addr.function); > + rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf); > + virUSBDeviceFree(usb); > + if (rc != 0) > + goto cleanup; > + break; > + } > > - virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend; > - if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || > - backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { > + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev; > + switch ((virMediatedDeviceModelType) mdevsrc->model) { > + case VIR_MDEV_MODEL_TYPE_VFIO_PCI: > + case VIR_MDEV_MODEL_TYPE_VFIO_AP: > + case VIR_MDEV_MODEL_TYPE_VFIO_CCW: > needsVfio = true; > - } > + break; > + case VIR_MDEV_MODEL_TYPE_LAST: > + default: > + virReportEnumRangeError(virMediatedDeviceModelType, > + mdevsrc->model); > + break; > + } > + break; > + } > > - if (pci == NULL) > - continue; > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { > + virPCIDevicePtr pci = virPCIDeviceNew( > + dev->source.subsys.u.pci.addr.domain, > + dev->source.subsys.u.pci.addr.bus, > + dev->source.subsys.u.pci.addr.slot, > + dev->source.subsys.u.pci.addr.function); > + > + virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend; > + if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || > + backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { > + needsVfio = true; > + } > > - rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf); > - virPCIDeviceFree(pci); > + if (pci == NULL) > + continue; > > - break; > - } > + rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf); > + virPCIDeviceFree(pci); > > - default: > - rc = 0; > - break; > - } /* switch */ > + break; > } > > + default: > + rc = 0; > + break; > + } /* switch */ > + } > + > for (i = 0; i < ctl->def->nfss; i++) { > - if (ctl->def->fss[i] && > - ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT && > + if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT && > (ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || > ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) && > ctl->def->fss[i]->src) { > @@ -1166,16 +1160,14 @@ get_files(vahControl * ctl) > } > > for (i = 0; i < ctl->def->ninputs; i++) { > - if (ctl->def->inputs[i] && > - ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { > + if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { > if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev, "rw") != 0) > goto cleanup; > } > } > > for (i = 0; i < ctl->def->nnets; i++) { > - if (ctl->def->nets[i] && > - ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && > + if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && > ctl->def->nets[i]->data.vhostuser) { > virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser; > > @@ -1186,8 +1178,7 @@ get_files(vahControl * ctl) > } > > for (i = 0; i < ctl->def->nmems; i++) { > - if (ctl->def->mems[i] && > - ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { > + if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { > if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) > goto cleanup; > } > -- > 2.24.0 > -- Jamie Strandboge | http://www.canonical.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/19/19 4:31 PM, Jamie Strandboge wrote: > On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > >> It was mentioned that the pointers in loops like: >> for (i = 0; i < ctl->def->nserials; i++) >> if (ctl->def->serials[i] ... >> will always be !NULL or we would have a much more serious problem. >> >> Simplify the if chains in get_files by dropping these checks. > > I don't really see why a NULL check is a problem so this feels a bit > like busy work and it seems short-circuiting and not doing is exactly > what you would want to do in the event of a 'much more serious problem'. > Is this really required? +0 to apply on principle (I've not reviewed the > changes closely). > If for example def->nserials and def->serials[i] disagree, then there is a serious bug somewhere. The internal API contract is that it should never happen, so code shouldn't check for the condition. I'm pretty sure if the XML parser is creating that situation, the qemu driver would crash in a dozen different places. So the patch isn't strictly required, but it is an improvement IMO: it reduces code, improves readability, and helps simplify review for other libvirt devs who may be confused by this uncommon idiom (I was). But I will leave it up to you guys to decide whether to push or not Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 20 Nov 2019, Cole Robinson wrote: > On 11/19/19 4:31 PM, Jamie Strandboge wrote: > > On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > > > >> It was mentioned that the pointers in loops like: > >> for (i = 0; i < ctl->def->nserials; i++) > >> if (ctl->def->serials[i] ... > >> will always be !NULL or we would have a much more serious problem. > >> > >> Simplify the if chains in get_files by dropping these checks. > > > > I don't really see why a NULL check is a problem so this feels a bit > > like busy work and it seems short-circuiting and not doing is exactly > > what you would want to do in the event of a 'much more serious problem'. > > Is this really required? +0 to apply on principle (I've not reviewed the > > changes closely). > > > > If for example def->nserials and def->serials[i] disagree, then there is > a serious bug somewhere. The internal API contract is that it should > never happen, so code shouldn't check for the condition. I'm pretty sure > if the XML parser is creating that situation, the qemu driver would > crash in a dozen different places. > > So the patch isn't strictly required, but it is an improvement IMO: it > reduces code, improves readability, and helps simplify review for other > libvirt devs who may be confused by this uncommon idiom (I was). But I > will leave it up to you guys to decide whether to push or not To be clear, I am intentionally not blocking on this. If, as upstream, you'd prefer this to be a certain way for reasons that outweigh my POV, by all means feel free to do that. The changes are mechanical and IMHO don't need an apparmor-focused review (though if you'd prefer I go through the full patch, I can). -- Jamie Strandboge | http://www.canonical.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/20/19 12:25 PM, Jamie Strandboge wrote: > On Wed, 20 Nov 2019, Cole Robinson wrote: > >> On 11/19/19 4:31 PM, Jamie Strandboge wrote: >>> On Thu, 14 Nov 2019, Christian Ehrhardt wrote: >>> >>>> It was mentioned that the pointers in loops like: >>>> for (i = 0; i < ctl->def->nserials; i++) >>>> if (ctl->def->serials[i] ... >>>> will always be !NULL or we would have a much more serious problem. >>>> >>>> Simplify the if chains in get_files by dropping these checks. >>> >>> I don't really see why a NULL check is a problem so this feels a bit >>> like busy work and it seems short-circuiting and not doing is exactly >>> what you would want to do in the event of a 'much more serious problem'. >>> Is this really required? +0 to apply on principle (I've not reviewed the >>> changes closely). >>> >> >> If for example def->nserials and def->serials[i] disagree, then there is >> a serious bug somewhere. The internal API contract is that it should >> never happen, so code shouldn't check for the condition. I'm pretty sure >> if the XML parser is creating that situation, the qemu driver would >> crash in a dozen different places. >> >> So the patch isn't strictly required, but it is an improvement IMO: it >> reduces code, improves readability, and helps simplify review for other >> libvirt devs who may be confused by this uncommon idiom (I was). But I >> will leave it up to you guys to decide whether to push or not > > To be clear, I am intentionally not blocking on this. If, as upstream, > you'd prefer this to be a certain way for reasons that outweigh my POV, > by all means feel free to do that. The changes are mechanical and IMHO > don't need an apparmor-focused review (though if you'd prefer I go > through the full patch, I can). > This patch moves the code to be more consistent with the rest of the libvirt code base, so I think it's good to push. I thought Christian was waiting on you for review before pushing apparmor patches though, I didn't want to step on any toes :) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 19, 2019 at 10:31 PM Jamie Strandboge <jamie@canonical.com> wrote: > > On Thu, 14 Nov 2019, Christian Ehrhardt wrote: > > > It was mentioned that the pointers in loops like: > > for (i = 0; i < ctl->def->nserials; i++) > > if (ctl->def->serials[i] ... > > will always be !NULL or we would have a much more serious problem. > > > > Simplify the if chains in get_files by dropping these checks. > > I don't really see why a NULL check is a problem so this feels a bit > like busy work and it seems short-circuiting and not doing is exactly > what you would want to do in the event of a 'much more serious problem'. > Is this really required? +0 to apply on principle (I've not reviewed the > changes closely). Well Cole brought it up and I intentionally wanted to have the discussion separate to the shmem changes. As I said in the mail on patch 2/2 in v1 of this I consider it "defensive programming" and not strictly required to be removed. I'm happy and fine to keep the checks as-is since they are not on a performance critical path. I agree as you say "in the event of a 'much more serious problem'" this would be exactly what we want. I'll withdraw this patch from the series then unless somebody really shows a reason to why we should drop the checks. > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > > --- > > src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------ > > 1 file changed, 63 insertions(+), 72 deletions(-) > > > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > > index c6c4bb9bd0..17f49a6259 100644 > > --- a/src/security/virt-aa-helper.c > > +++ b/src/security/virt-aa-helper.c > > @@ -965,8 +965,7 @@ get_files(vahControl * ctl) > > } > > > > for (i = 0; i < ctl->def->nserials; i++) > > - if (ctl->def->serials[i] && > > - (ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > + if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > > ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > > @@ -980,8 +979,7 @@ get_files(vahControl * ctl) > > goto cleanup; > > > > for (i = 0; i < ctl->def->nconsoles; i++) > > - if (ctl->def->consoles[i] && > > - (ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > + if ((ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > > ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > > @@ -993,8 +991,7 @@ get_files(vahControl * ctl) > > goto cleanup; > > > > for (i = 0; i < ctl->def->nparallels; i++) > > - if (ctl->def->parallels[i] && > > - (ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > + if ((ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > > ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > > @@ -1008,8 +1005,7 @@ get_files(vahControl * ctl) > > goto cleanup; > > > > for (i = 0; i < ctl->def->nchannels; i++) > > - if (ctl->def->channels[i] && > > - (ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > + if ((ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY || > > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV || > > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE || > > ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX || > > @@ -1082,76 +1078,74 @@ get_files(vahControl * ctl) > > "r") != 0) > > goto cleanup; > > > > - for (i = 0; i < ctl->def->nhostdevs; i++) > > - if (ctl->def->hostdevs[i]) { > > - virDomainHostdevDefPtr dev = ctl->def->hostdevs[i]; > > - virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; > > - switch (dev->source.subsys.type) { > > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { > > - virUSBDevicePtr usb = > > - virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL); > > + for (i = 0; i < ctl->def->nhostdevs; i++) { > > + virDomainHostdevDefPtr dev = ctl->def->hostdevs[i]; > > + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; > > + switch (dev->source.subsys.type) { > > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { > > + virUSBDevicePtr usb = > > + virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL); > > > > - if (usb == NULL) > > - continue; > > - > > - if (virHostdevFindUSBDevice(dev, true, &usb) < 0) > > - continue; > > + if (usb == NULL) > > + continue; > > > > - rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf); > > - virUSBDeviceFree(usb); > > - if (rc != 0) > > - goto cleanup; > > - break; > > - } > > + if (virHostdevFindUSBDevice(dev, true, &usb) < 0) > > + continue; > > > > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { > > - virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev; > > - switch ((virMediatedDeviceModelType) mdevsrc->model) { > > - case VIR_MDEV_MODEL_TYPE_VFIO_PCI: > > - case VIR_MDEV_MODEL_TYPE_VFIO_AP: > > - case VIR_MDEV_MODEL_TYPE_VFIO_CCW: > > - needsVfio = true; > > - break; > > - case VIR_MDEV_MODEL_TYPE_LAST: > > - default: > > - virReportEnumRangeError(virMediatedDeviceModelType, > > - mdevsrc->model); > > - break; > > - } > > - break; > > - } > > - > > - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { > > - virPCIDevicePtr pci = virPCIDeviceNew( > > - dev->source.subsys.u.pci.addr.domain, > > - dev->source.subsys.u.pci.addr.bus, > > - dev->source.subsys.u.pci.addr.slot, > > - dev->source.subsys.u.pci.addr.function); > > + rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf); > > + virUSBDeviceFree(usb); > > + if (rc != 0) > > + goto cleanup; > > + break; > > + } > > > > - virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend; > > - if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || > > - backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { > > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { > > + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &dev->source.subsys.u.mdev; > > + switch ((virMediatedDeviceModelType) mdevsrc->model) { > > + case VIR_MDEV_MODEL_TYPE_VFIO_PCI: > > + case VIR_MDEV_MODEL_TYPE_VFIO_AP: > > + case VIR_MDEV_MODEL_TYPE_VFIO_CCW: > > needsVfio = true; > > - } > > + break; > > + case VIR_MDEV_MODEL_TYPE_LAST: > > + default: > > + virReportEnumRangeError(virMediatedDeviceModelType, > > + mdevsrc->model); > > + break; > > + } > > + break; > > + } > > > > - if (pci == NULL) > > - continue; > > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { > > + virPCIDevicePtr pci = virPCIDeviceNew( > > + dev->source.subsys.u.pci.addr.domain, > > + dev->source.subsys.u.pci.addr.bus, > > + dev->source.subsys.u.pci.addr.slot, > > + dev->source.subsys.u.pci.addr.function); > > + > > + virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend; > > + if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || > > + backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { > > + needsVfio = true; > > + } > > > > - rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf); > > - virPCIDeviceFree(pci); > > + if (pci == NULL) > > + continue; > > > > - break; > > - } > > + rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf); > > + virPCIDeviceFree(pci); > > > > - default: > > - rc = 0; > > - break; > > - } /* switch */ > > + break; > > } > > > > + default: > > + rc = 0; > > + break; > > + } /* switch */ > > + } > > + > > for (i = 0; i < ctl->def->nfss; i++) { > > - if (ctl->def->fss[i] && > > - ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT && > > + if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT && > > (ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || > > ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) && > > ctl->def->fss[i]->src) { > > @@ -1166,16 +1160,14 @@ get_files(vahControl * ctl) > > } > > > > for (i = 0; i < ctl->def->ninputs; i++) { > > - if (ctl->def->inputs[i] && > > - ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { > > + if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { > > if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev, "rw") != 0) > > goto cleanup; > > } > > } > > > > for (i = 0; i < ctl->def->nnets; i++) { > > - if (ctl->def->nets[i] && > > - ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && > > + if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && > > ctl->def->nets[i]->data.vhostuser) { > > virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser; > > > > @@ -1186,8 +1178,7 @@ get_files(vahControl * ctl) > > } > > > > for (i = 0; i < ctl->def->nmems; i++) { > > - if (ctl->def->mems[i] && > > - ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { > > + if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { > > if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) > > goto cleanup; > > } > > -- > > 2.24.0 > > > -- > Jamie Strandboge | http://www.canonical.com -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.