[libvirt] [PATCH] conf: Parse guestfwd channel device info again
https://bugzilla.redhat.com/show_bug.cgi?id=1610072 Due to historical reasons we were not parsing device info on guestfwd channel. Sure, it doesn't make much sense to parse <address/> but it surely makes sense to parse its alias (which might be an user alias). This reverts commit 47a3dd46ead20e6fdc30bcdc1b8e707e250d33da which fixed https://bugzilla.redhat.com/show_bug.cgi?id=1172526. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3c254801cd..123abec1ba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12807,14 +12807,8 @@ virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt, } } - if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) { - VIR_DEBUG("Ignoring device address for gustfwd channel"); - } else if (virDomainDeviceInfoParseXML(xmlopt, node, - &def->info, flags) < 0) { + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; - } - if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && def->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB && -- 2.16.4
On Mon, 2018-08-20 at 18:52 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1610072
Due to historical reasons we were not parsing device info on
You spelled "hysterical raisins" wrong ;) [...]
- if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) { - VIR_DEBUG("Ignoring device address for gustfwd channel"); - } else if (virDomainDeviceInfoParseXML(xmlopt, node, - &def->info, flags) < 0) { + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; - } -
I agree that fixing Bug 1610072 is more important than preventing Bug 1172526 from showing up again, but it would be great if we could make it so both are fixed... How about parsing the info and then clearing out the address only if it's a guestfwd channel? The existing virDomainDeviceInfoClear() is a bit too thorough, but perhaps you can introduce a new virDomainDeviceInfoClearAddress() that only zeroes out the address and use that here. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, 2018-08-21 at 11:01 +0200, Andrea Bolognani wrote:
On Mon, 2018-08-20 at 18:52 +0200, Michal Privoznik wrote: [...]
- if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) { - VIR_DEBUG("Ignoring device address for gustfwd channel"); - } else if (virDomainDeviceInfoParseXML(xmlopt, node, - &def->info, flags) < 0) { + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; - } -
I agree that fixing Bug 1610072 is more important than preventing Bug 1172526 from showing up again, but it would be great if we could make it so both are fixed...
How about parsing the info and then clearing out the address only if it's a guestfwd channel? The existing virDomainDeviceInfoClear() is a bit too thorough, but perhaps you can introduce a new virDomainDeviceInfoClearAddress() that only zeroes out the address and use that here.
Given the issues with the approach I proposed, let's just cut our losses and merge your original attempt instead. Sorry for wasting your time :( Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On 08/21/2018 02:15 PM, Andrea Bolognani wrote:
On Tue, 2018-08-21 at 11:01 +0200, Andrea Bolognani wrote:
On Mon, 2018-08-20 at 18:52 +0200, Michal Privoznik wrote: [...]
- if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) { - VIR_DEBUG("Ignoring device address for gustfwd channel"); - } else if (virDomainDeviceInfoParseXML(xmlopt, node, - &def->info, flags) < 0) { + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; - } -
I agree that fixing Bug 1610072 is more important than preventing Bug 1172526 from showing up again, but it would be great if we could make it so both are fixed...
How about parsing the info and then clearing out the address only if it's a guestfwd channel? The existing virDomainDeviceInfoClear() is a bit too thorough, but perhaps you can introduce a new virDomainDeviceInfoClearAddress() that only zeroes out the address and use that here.
Given the issues with the approach I proposed, let's just cut our losses and merge your original attempt instead. Sorry for wasting your time :(
No worries.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Pushed thanks. Michal
participants (2)
-
Andrea Bolognani -
Michal Privoznik