Re: [libvirt] [PATCH] qemu: Generate agent socket path if missing
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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)