On Wed, Oct 02, 2024 at 01:00:33PM +0200, Martin Kletzander wrote:
static int testDomainAttachHostDevice(testDriver *driver, virDomainObj *vm, @@ -10144,28 +10232,68 @@ testDomainAttachDeviceLive(virDomainObj *vm, testDriver *driver) { const char *alias = NULL; + int ret = -1;
- if (dev->type != VIR_DOMAIN_DEVICE_HOSTDEV) { + switch (dev->type) { + case VIR_DOMAIN_DEVICE_NET: + testDomainObjCheckNetTaint(vm, dev->data.net); + ret = testDomainAttachNetDevice(driver, vm, dev->data.net); + if (!ret) {
if this is an integer where 0 is success and negative number is an error we tend to check for (ret == 0). In this case it does not do much, but in other instances it allows us to add different success return values in the future. And keeping the same way of doing things makes it nicer as consistent appearance is easier to read.
Isn't it better to be consistent with the original code? This came directly from qemu_hotplug.c
+ alias = dev->data.hostdev->info->alias; + dev->data.hostdev = NULL;
And since these two lines are the same for both code paths you can do that after the switch when you error out early ...
ditto, I think? regards john