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