[libvirt] [PATCH v2] Do not generate security_model when fs driver is anything but 'path'

QEMU does not support security_model for anything but 'path' fs driver type. Currently in libvirt, when security_model ( accessmode attribute) is not specified it auto-generates it irrespective of the fs driver type, which can result in a qemu error for drivers other than path. This patch ensures that the qemu cmdline is correctly generated by taking into account the fs driver type. Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com> --- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained. src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea1b763..e3a509a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2098,13 +2098,24 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } virBufferAdd(&opt, driver, -1); - if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { - virBufferAddLit(&opt, ",security_model=mapped"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { - virBufferAddLit(&opt, ",security_model=passthrough"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { - virBufferAddLit(&opt, ",security_model=none"); + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always */ + /* be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + } } + virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src);

On Tue, Jan 10, 2012 at 03:35:26PM +0530, Deepak C Shetty wrote:
QEMU does not support security_model for anything but 'path' fs driver type. Currently in libvirt, when security_model ( accessmode attribute) is not specified it auto-generates it irrespective of the fs driver type, which can result in a qemu error for drivers other than path. This patch ensures that the qemu cmdline is correctly generated by taking into account the fs driver type.
Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com> --- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained.
src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)
ACK 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 :|

On 01/10/2012 04:17 PM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 03:35:26PM +0530, Deepak C Shetty wrote:
--- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained.
src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) ACK
Daniel My sincere apologies, forgot to add 'goto error' after qemuReportError in the patch sent.
Putting the correct patch below...pls consider this. diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f2e9cfa..73f673f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2100,13 +2100,25 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } virBufferAdd(&opt, driver, -1); - if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { - virBufferAddLit(&opt, ",security_model=mapped"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { - virBufferAddLit(&opt, ",security_model=passthrough"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { - virBufferAddLit(&opt, ",security_model=none"); + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always */ + /* be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } } + virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src);

On Tue, Jan 10, 2012 at 06:23:31PM +0530, Deepak C Shetty wrote:
On 01/10/2012 04:17 PM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 03:35:26PM +0530, Deepak C Shetty wrote:
--- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained.
src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) ACK
Daniel My sincere apologies, forgot to add 'goto error' after qemuReportError in the patch sent.
Putting the correct patch below...pls consider this.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f2e9cfa..73f673f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2100,13 +2100,25 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } virBufferAdd(&opt, driver, -1);
- if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { - virBufferAddLit(&opt, ",security_model=mapped"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { - virBufferAddLit(&opt, ",security_model=passthrough"); - } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { - virBufferAddLit(&opt, ",security_model=none"); + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always */ + /* be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } } + virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src);
ACK to this version 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 :|

On 01/10/2012 06:08 AM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 06:23:31PM +0530, Deepak C Shetty wrote:
On 01/10/2012 04:17 PM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 03:35:26PM +0530, Deepak C Shetty wrote:
--- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained.
src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) ACK
ACK to this version
Now pushed, after squashing this in to fix the 'make check' failure. I also added Deepak to AUTHORS; let me know if I need to update anything for preferred spellings. diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index a7c0b5b..d051305 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -2142,8 +2142,8 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, virBufferAddLit(&opt, ",security_model=none"); } } else { - /* For other fs drivers, default(passthru) should always */ - /* be supported */ + /* For other fs drivers, default(passthru) should always + * be supported */ if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only supports passthrough accessmode")); diff --git i/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args w/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args index 4c498ba..8579810 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args +++ w/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args @@ -4,7 +4,10 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -fsdev local,security_model=passthrough,id=fsdev-fs0,\ path=/export/to/guest -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,\ mount_tag=/import/from/host,bus=pci.0,addr=0x3 \ --fsdev handle,security_model=mapped,id=fsdev-fs1,\ +-fsdev local,security_model=mapped,id=fsdev-fs1,\ path=/export/to/guest2 -device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,\ mount_tag=/import/from/host2,bus=pci.0,addr=0x4 \ --usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 +-fsdev handle,id=fsdev-fs2,\ +path=/export/to/guest3 -device virtio-9p-pci,id=fs2,fsdev=fsdev-fs2,\ +mount_tag=/import/from/host3,bus=pci.0,addr=0x5 \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git i/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml w/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml index 7ef5923..07d7e8a 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml +++ w/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml @@ -25,9 +25,14 @@ <target dir='/import/from/host'/> </filesystem> <filesystem accessmode='mapped'> - <driver type='handle'/> + <driver type='path'/> <source dir='/export/to/guest2'/> <target dir='/import/from/host2'/> </filesystem> + <filesystem> + <driver type='handle'/> + <source dir='/export/to/guest3'/> + <target dir='/import/from/host3'/> + </filesystem> </devices> </domain> -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/12/2012 02:18 AM, Eric Blake wrote:
On 01/10/2012 06:08 AM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 06:23:31PM +0530, Deepak C Shetty wrote:
On 01/10/2012 04:17 PM, Daniel P. Berrange wrote:
On Tue, Jan 10, 2012 at 03:35:26PM +0530, Deepak C Shetty wrote:
--- v2: - removed xml accessmode changes as suggested by dan. - every fs driver having a default accessmode always, retained.
src/qemu/qemu_command.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) ACK
ACK to this version Now pushed, after squashing this in to fix the 'make check' failure. I also added Deepak to AUTHORS; let me know if I need to update anything for preferred spellings.
Thanks Eric.
participants (3)
-
Daniel P. Berrange
-
Deepak C Shetty
-
Eric Blake