On 5/29/20 5:52 PM, Yi Wang wrote:
From: Liao Pingfang <liao.pingfang@zte.com.cn>
According to the context, here we are checking net->downscript's validity,
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> --- src/qemu/qemu_hotplug.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d1a2be1..8c99dfc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4672,9 +4672,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virDomainNetReleaseActualDevice(conn, vm->def, net); else VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); - } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET) { - if (net->script) - virNetDevRunEthernetScript(net->ifname, net->downscript); + } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) { + virNetDevRunEthernetScript(net->ifname, net->downscript); } virDomainNetDefFree(net); return 0;
I'm not quite sure why this change is needed and the commit message doesn't explain it well. My idea, when merging the original patch, was to have the following pattern: if (net->type == VIR_DOMAIN_NET_TYPE_NET) { } else if (net->type == VIR_DOMAAIN_NET_TYPE_ETHERNET) { } else if (net->type == ...) { } because it's easily extensible (e.g. if another action needs to be taken for say ethernet type interface then all that's needed is to call a function from corresponding if(). If your patch is merged then it needs to be (effectively) reverted. Can you elaborate on this please? Michal