Re: [PATCH 1/2] test_driver: provide basic NIC hotplug support

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