On 10/25/2016 08:42 AM, Pavel Hrdina wrote:
> On Mon, Oct 24, 2016 at 06:46:17PM -0400, John Ferlan wrote:
>> Commit id '2c32237' added the TLS object removal to the DetachChrDevice
>> all when it should have been added to the RemoveChrDevice since that's
>> the norm for similar processing (e.g. disk)
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_hotplug.c | 44 +++++++++++++++++++++-----------------------
>> 1 file changed, 21 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index cf69945..31b5876 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3549,7 +3549,9 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
>> virDomainChrDefPtr chr)
>> {
>> virObjectEventPtr event;
>> + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> char *charAlias = NULL;
>> + char *tlsAlias = NULL;
>> qemuDomainObjPrivatePtr priv = vm->privateData;
>> int ret = -1;
>> int rc;
>> @@ -3560,7 +3562,16 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
>> if (!(charAlias = qemuAliasChardevFromDevAlias(chr->info.alias)))
>> goto cleanup;
>>
>> + if (chr->source->type == VIR_DOMAIN_CHR_TYPE_TCP &&
>> + chr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES &&
>> + !(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>> + goto cleanup;
>> +
>> qemuDomainObjEnterMonitor(driver, vm);
>> +
>> + if (tlsAlias && qemuMonitorDelObject(priv->mon, tlsAlias) < 0)
>> + goto exit_monitor;
>> +
>
> Those two qemu commands should be swapped as well. This is similar to the
> issues that the next patch fixes. The chardev uses tls object so the chardev
> should be detached before the tls object.
Took out my trusty pen and paper today and drew a large chart of attach
order, error order, and detach/remove order.
Long story short, for this one yes, moving the TLS deletion after the
chardev deletion is proper due to dependencies.
I've pushed this change.
John
>
> ACK with that fixed.
>
> Pavel
>
>> rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
>> if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> goto cleanup;
>> @@ -3579,7 +3590,13 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
>>
>> cleanup:
>> VIR_FREE(charAlias);
>> + VIR_FREE(tlsAlias);
>> + virObjectUnref(cfg);
>> return ret;
>> +
>> + exit_monitor:
>> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
>> + goto cleanup;
>> }
>>
>>
>> @@ -4461,13 +4478,10 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> virDomainChrDefPtr chr)
>> {
>> int ret = -1;
>> - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> qemuDomainObjPrivatePtr priv = vm->privateData;
>> virDomainDefPtr vmdef = vm->def;
>> virDomainChrDefPtr tmpChr;
>> - char *objAlias = NULL;
>> char *devstr = NULL;
>> - char *charAlias = NULL;
>>
>> if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
>> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> @@ -4480,26 +4494,16 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>>
>> sa_assert(tmpChr->info.alias);
>>
>> - if (!(charAlias = qemuAliasChardevFromDevAlias(tmpChr->info.alias)))
>> - goto cleanup;
>> -
>> - if (tmpChr->source->type == VIR_DOMAIN_CHR_TYPE_TCP &&
>> - tmpChr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES &&
>> - !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>> - goto cleanup;
>> -
>> if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
>> goto cleanup;
>>
>> qemuDomainMarkDeviceForRemoval(vm, &tmpChr->info);
>>
>> qemuDomainObjEnterMonitor(driver, vm);
>> - if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0)
>> - goto exit_monitor;
>> -
>> - if (objAlias && qemuMonitorDelObject(priv->mon, objAlias) < 0)
>> - goto exit_monitor;
>> -
>> + if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) {
>> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
>> + goto cleanup;
>> + }
>> if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> goto cleanup;
>>
>> @@ -4511,13 +4515,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> cleanup:
>> qemuDomainResetDeviceRemoval(vm);
>> VIR_FREE(devstr);
>> - VIR_FREE(charAlias);
>> - virObjectUnref(cfg);
>> return ret;
>> -
>> - exit_monitor:
>> - ignore_value(qemuDomainObjExitMonitor(driver, vm));
>> - goto cleanup;
>> }
>>
>>
>> --
>> 2.7.4
>>
>> --
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list