[libvirt] [PATCH] qemu: Generate agent socket path if missing

[Eric Blake]

On 04/23/2013 09:40 AM, Jiri Denemark wrote:
> On Tue, Apr 09, 2013 at 19:05:28 +0200, Michal Privoznik wrote:
>> It's not desired to force users imagine path for a socket they
>> are not even supposed to connect to. On the other hand, we
>> already have a release where the qemu agent socket path is
>> exposed to XML, so we cannot silently drop it from there.
>> The new path is generated in form:
>>
>> $LOCALSTATEDIR/lib/libvirt/qemu/channel/target/$domain.$name
>> ---

I like the idea of this patch, but think we probably also ought to
document this choice of auto-generated path in formatdomain.html.in somehow.

>>  libvirt.spec.in        |  1 +
>>  src/Makefile.am        |  1 +
>>  src/conf/domain_conf.c | 34 ++++++++++++++++------------------
>>  src/qemu/qemu_domain.c | 16 ++++++++++++++++
>>  4 files changed, 34 insertions(+), 18 deletions(-)

>>  
>> +    /* auto generate unix socket path */
>> +    if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
>> +        dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>> +        dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>> +        dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>> +        !dev->data.chr->source.data.nix.path &&
>> +        (cfg || (driver && (cfg = virQEMUDriverGetConfig(driver))))) {
> 
> I think this should fail if path is NULL and
> (cfg || (driver && (cfg = virQEMUDriverGetConfig(driver)))) is not true.
> 
> On the other hand, do we actually need to check for this? Aren't both
> cfg and driver guaranteed to be non-NULL at this point?

'driver' corresponds to the opaque argument; we already have other
examples in the function that assume it can be NULL on some callback paths:

    /* set default disk types and drivers */
    if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
        virDomainDiskDefPtr disk = dev->data.disk;

        /* both of these require data from the driver config */
        if (driver && (cfg = virQEMUDriverGetConfig(driver))) {

so this is just matching existing style.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130423/cc0da312/attachment-0001.sig>