Re: [PATCH v11 3/5] qemu: Need to remove TLS object in RemoveRNGDevice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/25/2016 12:04 PM, Pavel Hrdina wrote:
> On Tue, Oct 25, 2016 at 09:19:58AM -0400, John Ferlan wrote:
>>
>>
>> On 10/25/2016 08:53 AM, Pavel Hrdina wrote:
>>> On Mon, Oct 24, 2016 at 06:46:19PM -0400, John Ferlan wrote:
>>>> Commit id '6e6b4bfc' added the object, but forgot the other end.
>>>>
>>>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>>>> ---
>>>>  src/qemu/qemu_hotplug.c | 12 +++++++++++-
>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> ACK if you agree with review on the first patch that the order of 
>>> qemuMonitorDelObject and qemuMonitorDetachCharDev should be swapped.
>>>
>>> Pavel
>>>
>>
>> I do agree, but I hadn't fully convinced myself - I needed the extra set
>> of eyes to help with that... Of course, I botched that again here by
>> removing the tlsobj before the chardev.   A problem which naturally
>> persists in patch 5, but is easily adjusted..
>>
>> I looked at and thought about qemuDomainRemoveRNGDevice long enough and
>> compared to qemuDomainRemoveDiskDevice, but couldn't quite make up my
>> mind and really didn't want to be creating "extra" patches for perhaps
>> all the erroneous removals.
>>
>> If my comparison is {Attach|Remove}Disk, which has...
>>
>> Attach: Add secobj, Add encobj, Add Drive, and Add Device where drive
>> can rely on secobj and/or encobj.  The encobj and secobj do not rely on
>> each other. The secobj is the possible secret for the iSCSI/RBD server,
>> while encobj is the possible secret for
>>
>> On attach error the removal is Del Drive, Del secobj, Del encobj (order
>> of secobj/encobj doesn't matter).
>>
>> Detach: Del Device and call RemoveDisk which Del secobj, Del encobj, and
>> Del Drive.
>>
>> Then, the RemoveDisk is probably wrong (Del Drive should be first), but
>> I didn't want to continue to bog down the chardev changes if my thoughts
>> in patch 2 were wrong.
> 
> I think that the RemoveDisk is wrong and that the Del Drive should be first.
> As you've wrote, the drive can rely on secobj and/or encobj so we should not
> remove the object while they can be still used.
> 

RemoveDisk is a different/separate issue - I sent a separate patch.

As for the rest of this - I think in my latest response to 2/5 - the
pencil and paper exercise shows the delete TLS object goes after the
Detach chardev below.

That then just leaves fixing up the order for the secret object in patch
5 of this series.

John

> Pavel
> 
>>
>> Long way of saying, I want to get it right for this path and then if I'm
>> right generate a RemoveDisk patch to swap order.
>>
>> John
>>
>>>>
>>>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>>>> index e287aaf..f401b48 100644
>>>> --- a/src/qemu/qemu_hotplug.c
>>>> +++ b/src/qemu/qemu_hotplug.c
>>>> @@ -3608,6 +3608,7 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>>>>      virObjectEventPtr event;
>>>>      char *charAlias = NULL;
>>>>      char *objAlias = NULL;
>>>> +    char *tlsAlias = NULL;
>>>>      qemuDomainObjPrivatePtr priv = vm->privateData;
>>>>      ssize_t idx;
>>>>      int ret = -1;
>>>> @@ -3616,15 +3617,23 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>>>>      VIR_DEBUG("Removing RNG device %s from domain %p %s",
>>>>                rng->info.alias, vm, vm->def->name);
>>>>  
>>>> +
>>>>      if (virAsprintf(&objAlias, "obj%s", rng->info.alias) < 0)
>>>>          goto cleanup;
>>>>  
>>>>      if (!(charAlias = qemuAliasChardevFromDevAlias(rng->info.alias)))
>>>>          goto cleanup;
>>>>  
>>>> +    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
>>>> +        !(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>>>> +        goto cleanup;
>>>> +
>>>>      qemuDomainObjEnterMonitor(driver, vm);
>>>> -    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
>>>> +    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
>>>> +        if (tlsAlias)
>>>> +            ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
>>>>          ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
>>>> +    }
>>>>  
>>>>      rc = qemuMonitorDelObject(priv->mon, objAlias);
>>>>  
>>>> @@ -3648,6 +3657,7 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>>>>   cleanup:
>>>>      VIR_FREE(charAlias);
>>>>      VIR_FREE(objAlias);
>>>> +    VIR_FREE(tlsAlias);
>>>>      return ret;
>>>>  }
>>>>  
>>>> -- 
>>>> 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]