[libvirt] [PATCH v11 1/5] qemu: Move TLS object remove from DetachChr to RemoveChr

[John Ferlan]

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 at redhat.com>
>> ---
>>  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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list