- Devel - libvirt List Archives

Re: [libvirt] [libvirt-glib 8/9] Connect a bunch of domain event to signals
by Daniel P. Berrange 29 Sep '11

29 Sep '11

On Thu, Sep 29, 2011 at 02:25:43PM +0300, Zeeshan Ali (Khattak) wrote: > On Thu, Sep 29, 2011 at 11:45 AM, Daniel P. Berrange > <berrange(a)redhat.com> wrote: > > On Wed, Sep 28, 2011 at 09:30:57PM +0200, Marc-André Lureau wrote: > >> --- > >> libvirt-gobject/libvirt-gobject-connection.c | 126 ++++++++++++++++++++++++++ > >> 1 files changed, 126 insertions(+), 0 deletions(-) > >> > >> diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c > >> index e99a08d..34781e6 100644 > >> --- a/libvirt-gobject/libvirt-gobject-connection.c > >> +++ b/libvirt-gobject/libvirt-gobject-connection.c > > > >> @@ -277,6 +398,10 @@ gboolean gvir_connection_open(GVirConnection *conn, > >> return FALSE; > >> } > >> > >> + if (virConnectDomainEventRegister(priv->conn, domain_event_cb, conn, NULL) == -1) { > >> + g_warning("Failed to register domain events, ignoring"); > >> + } > >> + > > > > You might have noticed that we jump through some horrific hoops in > > gvir_connection_fetch_domains(). The reason for this is so that we > > can issue fake event notifications for start/stopped/added/removed > > when virConnectDomainEventRegister() is not available. Not all hypervisors > > support the virConnectDomainEventRegister() API, so we need that fallback > > code. When you succesfully call virConnectDomainEventRegister(), we should > > disable the event emitting part at the end of gvir_connection_fetch_domains() > > to avoid duplicate events. > > In that case, create_domain() method should emit the signal too when > event didn't register? If 'create_domain' updates the hash table directly with the new guest object, then, yes it should emit a signal. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

1 0

[libvirt] [libvirt-glib 8/9] Connect a bunch of domain event to signals
by Zeeshan Ali (Khattak) 29 Sep '11

29 Sep '11

Forgot to hit 'reply-all' :( ---------- Forwarded message ---------- From: Zeeshan Ali (Khattak) <zeeshanak(a)gnome.org> Date: Thu, Sep 29, 2011 at 2:25 PM Subject: Re: [libvirt] [libvirt-glib 8/9] Connect a bunch of domain event to signals To: "Daniel P. Berrange" <berrange(a)redhat.com> On Thu, Sep 29, 2011 at 11:45 AM, Daniel P. Berrange <berrange(a)redhat.com> wrote: > On Wed, Sep 28, 2011 at 09:30:57PM +0200, Marc-André Lureau wrote: >> --- >> libvirt-gobject/libvirt-gobject-connection.c | 126 ++++++++++++++++++++++++++ >> 1 files changed, 126 insertions(+), 0 deletions(-) >> >> diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c >> index e99a08d..34781e6 100644 >> --- a/libvirt-gobject/libvirt-gobject-connection.c >> +++ b/libvirt-gobject/libvirt-gobject-connection.c > >> @@ -277,6 +398,10 @@ gboolean gvir_connection_open(GVirConnection *conn, >> return FALSE; >> } >> >> + if (virConnectDomainEventRegister(priv->conn, domain_event_cb, conn, NULL) == -1) { >> + g_warning("Failed to register domain events, ignoring"); >> + } >> + > > You might have noticed that we jump through some horrific hoops in > gvir_connection_fetch_domains(). The reason for this is so that we > can issue fake event notifications for start/stopped/added/removed > when virConnectDomainEventRegister() is not available. Not all hypervisors > support the virConnectDomainEventRegister() API, so we need that fallback > code. When you succesfully call virConnectDomainEventRegister(), we should > disable the event emitting part at the end of gvir_connection_fetch_domains() > to avoid duplicate events. In that case, create_domain() method should emit the signal too when event didn't register? -- Regards, Zeeshan Ali (Khattak) FSF member#5124

1 0

[libvirt] [libvirt-glib 1/9] doc: add some missing return value documentation
by Marc-André Lureau 29 Sep '11

29 Sep '11

From: Marc-André Lureau <marcandre.lureau(a)gmail.com> --- libvirt-gobject/libvirt-gobject-stream.c | 1 + libvirt-gobject/libvirt-gobject-stream.h | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c index 255ad73..519d733 100644 --- a/libvirt-gobject/libvirt-gobject-stream.c +++ b/libvirt-gobject/libvirt-gobject-stream.c @@ -191,6 +191,7 @@ stream_sink(virStreamPtr st G_GNUC_UNUSED, * @stream: the stream * @func: (scope notified): the callback for writing data to application * @user_data: (closure): data to be passed to @callback + * Returns: the number of bytes consumed or -1 upon error * * Receive the entire data stream, sending the data to the * requested data sink. This is simply a convenient alternative diff --git a/libvirt-gobject/libvirt-gobject-stream.h b/libvirt-gobject/libvirt-gobject-stream.h index 27fa8b7..fd9c9bb 100644 --- a/libvirt-gobject/libvirt-gobject-stream.h +++ b/libvirt-gobject/libvirt-gobject-stream.h @@ -65,6 +65,8 @@ struct _GVirStreamClass * @buf: data pointer * @nbytes: data size * @user_data: user data passed to the function + * Returns: the number of bytes filled, 0 upon end + * of file, or -1 upon error */ typedef gint (* GVirStreamSinkFunc) (GVirStream *stream, const gchar *buf, -- 1.7.6.2

2 11

[libvirt] [PATCH] hyperv: Report an error for acceptable URI schemes with a transport
by Matthias Bolte 29 Sep '11

29 Sep '11

Before, URIs such as hyperv+ssh:// have been declined by the Hyper-V driver resulting in the remote driver trying to connect to an non-existing libvirtd. Now such URIs trigger an error in the Hyper-V driver suggesting to try again without the transport part in the scheme. --- src/hyperv/hyperv_driver.c | 25 ++++++++++++++++++++++--- 1 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index b022fee..c26d29f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -69,6 +69,7 @@ static virDrvOpenStatus hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; + char *plus; hypervPrivate *priv = NULL; char *username = NULL; char *password = NULL; @@ -77,12 +78,30 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); - /* Decline if the URI is NULL or the scheme is not hyperv */ - if (conn->uri == NULL || conn->uri->scheme == NULL || - STRCASENEQ(conn->uri->scheme, "hyperv")) { + /* Decline if the URI is NULL or the scheme is NULL */ + if (conn->uri == NULL || conn->uri->scheme == NULL) { return VIR_DRV_OPEN_DECLINED; } + /* Decline if the scheme is not hyperv */ + plus = strchr(conn->uri->scheme, '+'); + + if (plus == NULL) { + if (STRCASENEQ(conn->uri->scheme, "hyperv")) { + return VIR_DRV_OPEN_DECLINED; + } + } else { + if (plus - conn->uri->scheme != 6 || + STRCASENEQLEN(conn->uri->scheme, "hyperv", 6)) { + return VIR_DRV_OPEN_DECLINED; + } + + HYPERV_ERROR(VIR_ERR_INVALID_ARG, "%s", + _("Transport in URI scheme is not supported, try again " + "without the transport part")); + return VIR_DRV_OPEN_ERROR; + } + /* Require server part */ if (conn->uri->server == NULL) { HYPERV_ERROR(VIR_ERR_INVALID_ARG, "%s", -- 1.7.4.1

2 2

[libvirt] [PATCH] esx: Report an error for acceptable URI schemes with a transport
by Matthias Bolte 29 Sep '11

29 Sep '11

Before, URIs such as esx+ssh:// have been declined by the ESX driver resulting in the remote driver trying to connect to an non-existing libvirtd. Now such URIs trigger and error in the ESX driver suggesting to try again without the transport part in the scheme. --- src/esx/esx_driver.c | 31 ++++++++++++++++++++++++++----- 1 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f1102ea..c81eca8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -938,20 +938,41 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; + char *plus; esxPrivate *priv = NULL; char *potentialVCenterIpAddress = NULL; char vCenterIpAddress[NI_MAXHOST] = ""; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); - /* Decline if the URI is NULL or the scheme is not one of {vpx|esx|gsx} */ - if (conn->uri == NULL || conn->uri->scheme == NULL || - (STRCASENEQ(conn->uri->scheme, "vpx") && - STRCASENEQ(conn->uri->scheme, "esx") && - STRCASENEQ(conn->uri->scheme, "gsx"))) { + /* Decline if the URI is NULL or the scheme is NULL */ + if (conn->uri == NULL || conn->uri->scheme == NULL) { return VIR_DRV_OPEN_DECLINED; } + /* Decline if the scheme is not one of {vpx|esx|gsx} */ + plus = strchr(conn->uri->scheme, '+'); + + if (plus == NULL) { + if (STRCASENEQ(conn->uri->scheme, "vpx") && + STRCASENEQ(conn->uri->scheme, "esx") && + STRCASENEQ(conn->uri->scheme, "gsx")) { + return VIR_DRV_OPEN_DECLINED; + } + } else { + if (plus - conn->uri->scheme != 3 || + (STRCASENEQLEN(conn->uri->scheme, "vpx", 3) && + STRCASENEQLEN(conn->uri->scheme, "esx", 3) && + STRCASENEQLEN(conn->uri->scheme, "gsx", 3))) { + return VIR_DRV_OPEN_DECLINED; + } + + ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", + _("Transport in URI scheme is not supported, try again " + "without the transport part")); + return VIR_DRV_OPEN_ERROR; + } + /* Require server part */ if (conn->uri->server == NULL) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", -- 1.7.4.1

2 2

[libvirt] [libvirt-glib 1/2] Actually provide gvir_storage_vol_get_path()
by Zeeshan Ali (Khattak) 29 Sep '11

29 Sep '11

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> This was only declared in header previously but not implemented. --- libvirt-gobject/libvirt-gobject-storage-vol.c | 11 +++++++++++ libvirt-gobject/libvirt-gobject.sym | 1 + 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index b94f6cf..6cbae84 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -182,6 +182,17 @@ const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol) return name; } +const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol) +{ + GVirStorageVolPrivate *priv = vol->priv; + const char *path; + + if (!(path = virStorageVolGetPath(priv->handle))) { + g_error("Failed to get storage_vol path on %p", priv->handle); + } + + return path; +} /** * gvir_storage_vol_get_config: diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index ff2f4cf..ea4ec38 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -83,6 +83,7 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_storage_vol_get_type; gvir_storage_vol_handle_get_type; gvir_storage_vol_get_name; + gvir_storage_vol_get_path; gvir_storage_vol_get_config; gvir_connection_handle_get_type; -- 1.7.6.2

2 3

Re: [libvirt] [fedora-virt] Guest can no longer be bootet with libvirt 0.9.6 from virt-preview repo
by Eric Blake 29 Sep '11

29 Sep '11

[adding libvirt list] On 09/28/2011 09:24 AM, Dennis Jacobfeuerborn wrote: >>>>> I just noticed that once I upgrade to libvirt-0.9.6-1 from the >>>>> virt-preview >>>>> repo on my Fedora 15 machine I can no longer boot guests that use virtio >>>>> disks. I only get "Boot failed: could not read the boot disk". >>>>> Removing the disk and re-adding it as IDE drive allows KVM to boot from >>>>> the >>>>> disk (although the full boot obviously fails due to the hda/vda naming >>>>> difference but that's expected). >>>>> >>>>> After downgrading to libvirt-0.8.8-7 again the problem goes away and the >>>>> guest boot fine from virtio disks. >>>>> >> >> Somewhere around 0.9.4 libvirt switched to use a different method to >> indicate from which device to boot. >> See http://libvirt.org/git/?p=libvirt.git;a=commit;h=c3068d4d2381146ed46051ad63… >> >> I think this causes Seabios to skip using extboot.bin for booting and >> to try to directly boot from a disk. >> If your Seabios version is too old, it won't support booting from >> virtio disks directly. >> > > The libvirt package should probably be updated to require seabios-bin>= 0.6.2. Yes, that probably needs to happen to libvirt.spec when built for F16 (and thus imported into F15 via virt-preview). I'll tackle that, since I've been making a couple other spec file patches lately. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

5 6

[libvirt] [test-API][PATCH 1/2] update xmlgenerator.py to support for spice graphics type
by Nan Zhang 29 Sep '11

29 Sep '11

This extends graphics element for spice XML composing, and support sub-elements settings for audio, images, streaming and so on: <graphics type='spice' autoport='yes'> <image compression='auto_glz'/> <jpeg compression='auto'/> <zlib compression='auto'/> <playback compression='on'/> <streaming mode='filter'/> <clipboard copypaste='no'/> </graphics> --- utils/Python/xmlbuilder.py | 9 ++++++- utils/Python/xmlgenerator.py | 51 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/utils/Python/xmlbuilder.py b/utils/Python/xmlbuilder.py index 5a0f8c8..3dbe576 100644 --- a/utils/Python/xmlbuilder.py +++ b/utils/Python/xmlbuilder.py @@ -297,6 +297,13 @@ if __name__ == "__main__": params['memory'] = '1048576' params['vcpu'] = '2' params['inputbus'] = 'usb' + params['graphtype'] = 'spice' + params['image'] = 'auto_glz' + params['jpeg'] = 'auto' + params['zlib'] = 'auto' + params['playback'] = 'on' + params['streaming'] = 'filter' + params['clipboard'] = 'no' params['sound'] = 'ac97' params['bootcd'] = '/iso/rhel5.iso' @@ -367,7 +374,7 @@ if __name__ == "__main__": #---------------------------------------- # get domain snapshot xml string #---------------------------------------- - params['name'] = 'hello' + params['snapshotname'] = 'hello' params['description'] = 'hello snapshot' snapshot_xml = xmlobj.build_domain_snapshot(params) diff --git a/utils/Python/xmlgenerator.py b/utils/Python/xmlgenerator.py index d57dd33..b61ceb1 100644 --- a/utils/Python/xmlgenerator.py +++ b/utils/Python/xmlgenerator.py @@ -235,9 +235,54 @@ def domain_xml(params, install = False): # <graphics> graphics_element = domain.createElement('graphics') - graphics_element.setAttribute('type', 'vnc') - graphics_element.setAttribute('port', '-1') - graphics_element.setAttribute('keymap', 'en-us') + if not params.has_key('graphtype'): + params['graphtype'] == 'vnc' + + graphics_element.setAttribute('type', params['graphtype']) + if params['graphtype'] == 'vnc': + graphics_element.setAttribute('port', '-1') + graphics_element.setAttribute('keymap', 'en-us') + elif params['graphtype'] == 'spice': + graphics_element.setAttribute('autoport', 'yes') + if params.has_key('image'): + image_element = domain.createElement('image') + # image to set image compression (accepts + # auto_glz, auto_lz, quic, glz, lz, off) + image_element.setAttribute('compression', params['image']) + graphics_element.appendChild(image_element) + if params.has_key('jpeg'): + jpeg_element = domain.createElement('jpeg') + # jpeg for JPEG compression for images over wan (accepts + # auto, never, always) + jpeg_element.setAttribute('compression', params['jpeg']) + graphics_element.appendChild(jpeg_element) + if params.has_key('zlib'): + zlib_element = domain.createElement('zlib') + # zlib for configuring wan image compression (accepts + # auto, never, always) + zlib_element.setAttribute('compression', params['zlib']) + graphics_element.appendChild(zlib_element) + if params.has_key('playback'): + playback_element = domain.createElement('playback') + # playback for enabling audio stream compression (accepts on or off) + playback_element.setAttribute('compression', params['playback']) + graphics_element.appendChild(playback_element) + if params.has_key('streaming'): + streaming_element = domain.createElement('streaming') + # streamming for settings it's mode attribute to one of + # filter, all or off + streaming_element.setAttribute('mode', params['streaming']) + graphics_element.appendChild(streaming_element) + if params.has_key('clipboard'): + clipboard_element = domain.createElement('clipboard') + # Copy & Paste functionality is enabled by default, and can + # be disabled by setting the copypaste property to no + clipboard_element.setAttribute('copypaste', params['clipboard']) + graphics_element.appendChild(clipboard_element) + else: + print 'Wrong graphics type was specified.' + sys.exit(1) + devices_element.appendChild(graphics_element) domain_element.appendChild(devices_element) -- 1.7.4.4

2 4

[libvirt] [test-API][PATCH] Add test case update_devflag.py for update device flag
by Nan Zhang 29 Sep '11

29 Sep '11

--- repos/domain/update_devflag.py | 163 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 163 insertions(+), 0 deletions(-) create mode 100644 repos/domain/update_devflag.py diff --git a/repos/domain/update_devflag.py b/repos/domain/update_devflag.py new file mode 100644 index 0000000..30f75b0 --- /dev/null +++ b/repos/domain/update_devflag.py @@ -0,0 +1,163 @@ +#!/usr/bin/evn python +"""Update virtual device to guest from an XML file +""" + +__author__ = 'Nan Zhang: nzhang(a)redhat.com' +__date__ = 'Fri Sep 2, 2011' +__version__ = '0.1.0' +__credits__ = 'Copyright (C) 2011 Red Hat, Inc.' +__all__ = ['usage', 'update_devflag'] + +import os +import re +import sys +import commands +from xml.dom import minidom + +def append_path(path): + """Append root path of package""" + if path in sys.path: + pass + else: + sys.path.append(path) + +pwd = os.getcwd() +result = re.search('(.*)libvirt-test-API', pwd) +append_path(result.group(0)) + +from lib import connectAPI +from lib import domainAPI +from utils.Python import utils +from utils.Python import xmlbuilder +from exception import LibvirtAPI + +def usage(): + print '''usage: mandatory arguments: + guestname + devtype + ''' + +def check_params(params): + """Verify inputing parameter dictionary""" + logger = params['logger'] + keys = ['guestname', 'devtype'] + for key in keys: + if key not in params: + logger.error("%s is required" %key) + usage() + return 1 + return 0 + +def create_image(params, img_name, img_size): + """Create an image file""" + logger = params['logger'] + stat, ret = commands.getstatusoutput("dd if=/dev/zero of=%s bs=1 \ + count=1 seek=%s" % (img_name, img_size)) + if stat == 0: + logger.debug("create image result:\n%s" % ret) + return True + else: + return False + +def check_updated_device(params, guestname, domobj, srcfile): + """Check if the device is updated""" + logger = params['logger'] + xmlobj = domobj.get_xml_desc(guestname) + domxml = minidom.parseString(xmlobj) + + for diskTag in domxml.getElementsByTagName("source"): + if diskTag.parentNode.getAttribute("device") == 'cdrom': + upfile = diskTag.getAttribute("file") + elif diskTag.parentNode.getAttribute('device') == 'floppy': + upfile = diskTag.getAttribute("file") + + if upfile == srcfile: + return False, upfile + else: + return True, upfile + +def update_devflag(params): + """Update virtual device to a domain from xml""" + + # Initiate and check parameters + params_check_result = check_params(params) + if params_check_result: + return 1 + logger = params['logger'] + guestname = params['guestname'] + devtype = params['devtype'] + if devtype == 'cdrom': + xmlargs = {} + xmlargs['guestname'] = guestname + xmlargs['guesttype'] = 'kvm' + xmlargs['hdmodel'] = 'ide' + xmlargs['bootcd'] = '/var/lib/libvirt/boot/cdrom.img' + srcfile = xmlargs['bootcd'] + create_image(params, srcfile, '100M') + elif devtype == 'floppy': + xmlargs = {} + xmlargs['guestname'] = guestname + xmlargs['floppysource'] = '/var/lib/libvirt/boot/floppy.img' + srcfile = xmlargs['floppysource'] + create_image(params, srcfile, '2M') + else: + srcfile = None + logger.error("Wrong device type was specified.") + return 1 + + if not params.has_key('flag'): + flag = domainAPI.VIR_DOMAIN_AFFECT_CONFIG + + # Connect to local hypervisor connection URI + util = utils.Utils() + uri = util.get_uri('127.0.0.1') + conn = connectAPI.ConnectAPI() + virconn = conn.open(uri) + + caps = conn.get_caps() + logger.debug(caps) + + # Generate device XML for updating + domobj = domainAPI.DomainAPI(virconn) + newxmlobj = xmlbuilder.XmlBuilder() + + if devtype == 'cdrom': + newdevxml = newxmlobj.build_cdrom(xmlargs) + elif devtype == 'floppy': + newdevxml = newxmlobj.build_floppy(xmlargs) + + logger.debug("block device xml desc:\n%s" %newdevxml) + + try: + try: + domobj.update_device_flag(guestname, newdevxml, flag) + res, upfile = check_updated_device(params, guestname, \ + domobj, srcfile) + if res: + logger.info("success to update '%s' device: %s\n" % \ + (devtype, upfile)) + else: + logger.error("fail to update '%s' device: %s\n" % \ + (devtype, upfile)) + except LibvirtAPI, e: + logger.error("API error message: %s, error code is %s" % + (e.response()['message'], e.response()['code'])) + conn.close() + logger.info("closed hypervisor connection") + return 1 + finally: + conn.close() + logger.info("closed hypervisor connection") + + return 0 + +def update_devflag_clean(params): + """Clean testing environment""" + logger = params['logger'] + + if params['devtype'] == 'cdrom': + os.system('rm -f /var/lib/libvirt/boot/cdrom.img') + elif params['devtype'] == 'floppy': + os.system('rm -f /var/lib/libvirt/boot/floppy.img') + else: + logger.debug("image file was not found.") -- 1.7.4.4

1 1

[libvirt] [PATCH] security: chown/label bidrectional and unidirectional fifos
by Laine Stump 29 Sep '11

29 Sep '11

This patch fixes the regression with using named pipes for qemu serial devices noted in: https://bugzilla.redhat.com/show_bug.cgi?id=740478 The problem was that, while new code in libvirt looks for a single bidirectional fifo of the name given in the config, then relabels that and continues without looking for / relabelling the two unidirectional fifos named ${name}.in and ${name}.out, qemu looks in the opposite order. So if the user had naively created all three fifos, libvirt would relabel the bidirectional fifo to allow qemu access, but qemu would attempt to use the two unidirectional fifos and fail (because it didn't have proper permissions/rights). The solution implemented here is to always look for and chown/relabel both types of fifos, then let qemu decide which one it wants to use (in the unusual case that both are present). If one of the types is present, libvirt will silently ignore when the other type is missing (since that will be the most common case), but if neither type is present, there will be an error logged about failing to relabel/chown one of the bidirectional pipes. (Likewise, if one direction of the unidirectional pipes is present but the other is missing, this will also result in an error log). (Note commit d37c6a3a (which first appeared in libvirt-0.9.2) added the code that checked for a bidirectional fifo. Prior to that commit, bidirectional fifos fdor serial devices didn't work unless the fifo was pre-owned/labelled such that qemu could access it.) --- src/security/security_dac.c | 39 ++++++++++++++++++++++++++++++--------- src/security/security_selinux.c | 39 ++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index af02236..f97d2d6 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -397,6 +397,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); char *in = NULL, *out = NULL; + bool found_bipipe = false; int ret = -1; switch (dev->type) { @@ -406,19 +407,28 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, break; case VIR_DOMAIN_CHR_TYPE_PIPE: + /* look for / chown both bidirectional pipe and dual uni-directional + * pipes if found; let the hypervisor decide which to use. + */ if (virFileExists(dev->data.file.path)) { + found_bipipe = true; if (virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group) < 0) goto done; - } else { - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; - } - if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || - (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) - goto done; } + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if (found_bipipe && + !(virFileExists(in) || virFileExists(out))) { + ret = 0; + goto done; + } + if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || + (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) + goto done; + ret = 0; break; @@ -438,6 +448,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr dev) { char *in = NULL, *out = NULL; + bool found_bipipe = false; int ret = -1; switch (dev->type) { @@ -447,11 +458,21 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, break; case VIR_DOMAIN_CHR_TYPE_PIPE: + if (virFileExists(dev->data.file.path)) { + found_bipipe = true; + if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path) < 0) + goto done; + } if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { virReportOOMError(); goto done; } + if (found_bipipe && + !(virFileExists(in) || virFileExists(out))) { + ret = 0; + goto done; + } if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) || (virSecurityDACRestoreSecurityFileLabel(in) < 0)) goto done; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0807a34..e7a18a6 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -794,6 +794,7 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; char *in = NULL, *out = NULL; + bool found_bipipe = false; int ret = -1; if (secdef->norelabel) @@ -806,19 +807,28 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm, break; case VIR_DOMAIN_CHR_TYPE_PIPE: + /* look for / chown both bidirectional pipe and dual uni-directional + * pipes if found; let the hypervisor decide which to use. + */ if (virFileExists(dev->data.file.path)) { + found_bipipe = true; if (SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel) < 0) goto done; - } else { - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; - } - if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || - (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) - goto done; } + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if (found_bipipe && + !(virFileExists(in) || virFileExists(out))) { + ret = 0; + goto done; + } + if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || + (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) + goto done; + ret = 0; break; @@ -840,6 +850,7 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; char *in = NULL, *out = NULL; + bool found_bipipe = false; int ret = -1; if (secdef->norelabel) @@ -853,11 +864,21 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm, ret = 0; break; case VIR_DOMAIN_CHR_TYPE_PIPE: + if (virFileExists(dev->data.file.path)) { + found_bipipe = true; + if (SELinuxRestoreSecurityFileLabel(dev->data.file.path) < 0) + goto done; + } if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { virReportOOMError(); goto done; } + if (found_bipipe && + !(virFileExists(in) || virFileExists(out))) { + ret = 0; + goto done; + } if ((SELinuxRestoreSecurityFileLabel(out) < 0) || (SELinuxRestoreSecurityFileLabel(in) < 0)) goto done; -- 1.7.3.4

3 4