[libvirt] [PATCH 2/2] storage: Generate correct parameters for CIFS

[Peter Krempa]

On Wed, Jun 03, 2015 at 13:06:58 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1186969
> 
> When generating the path to the dir for a CIFS/Samba driver, the code
> would generate a source path for the mount using "%s:%s" while the
> mount.cifs expects to see "//%s/%s". So check for the cifsfs and
> format the source path appropriately.
> 
> Additionally, since there is no means to authenticate, the mount
> needs a "-o guest" on the command line in order to anonymously mount
> the Samba directory.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  docs/formatstorage.html.in       |  7 +++++--
>  docs/storage.html.in             |  3 ++-
>  src/storage/storage_backend_fs.c | 29 ++++++++++++++++++++++++-----
>  3 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 17558f8..b6f4361 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -124,11 +124,14 @@
>          <span class="since">Since 0.4.1</span></dd>
>        <dt><code>dir</code></dt>
>        <dd>Provides the source for pools backed by directories (pool
> -        type <code>dir</code>), or optionally to select a subdirectory
> +        types <code>dir</code>, <code>netfs</code>, <code>gluster</code>),
> +        or optionally to select a subdirectory
>          within a pool that resembles a filesystem (pool
>          type <code>gluster</code>). May
>          only occur once. Contains a single attribute <code>path</code>
> -        which is the fully qualified path to the backing directory.
> +        which is the fully qualified path to the backing directory or
> +        for a <code>netfs</code> pool type using <code>format</code>
> +        type "cifs", the path to the Samba share without the leading slash.
>          <span class="since">Since 0.4.1</span></dd>
>        <dt><code>adapter</code></dt>
>        <dd>Provides the source for pools backed by SCSI adapters (pool
> diff --git a/docs/storage.html.in b/docs/storage.html.in
> index 92e9ae7..0b467d5 100644
> --- a/docs/storage.html.in
> +++ b/docs/storage.html.in
> @@ -291,7 +291,8 @@
>          the <a href="#StorageBackendGluster">gluster</a> pool.)
>        </li>
>        <li>
> -        <code>cifs</code> - use the SMB (samba) or CIFS file system
> +        <code>cifs</code> - use the SMB (samba) or CIFS file system.
> +        The mount will use "-o guest" to mount the directory anonymously.
>        </li>
>      </ul>
>  
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 337b8d3..6ba698c 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -388,6 +388,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
>                      pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO);
>      bool glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS &&
>                        pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS);
> +    bool cifsfs = (pool->def->type == VIR_STORAGE_POOL_NETFS &&
> +                   pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS);

@cifsfs can be true only if pool->def->type == VIR_STORAGE_POOL_NETFS.

>      virCommandPtr cmd = NULL;
>      int ret = -1;
>      int rc;
> @@ -427,11 +429,17 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
>      }
>  
>      if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
> -        if (virAsprintf(&src, "%s:%s",
> -                        pool->def->source.hosts[0].name,
> -                        pool->def->source.dir) == -1)
> -            return -1;
> -
> +        if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
> +            if (virAsprintf(&src, "//%s/%s",
> +                            pool->def->source.hosts[0].name,
> +                            pool->def->source.dir) == -1)
> +                return -1;
> +        } else {
> +            if (virAsprintf(&src, "%s:%s",
> +                            pool->def->source.hosts[0].name,
> +                            pool->def->source.dir) == -1)
> +                return -1;
> +        }
>      } else {
>          if (VIR_STRDUP(src, pool->def->source.devices[0].path) < 0)
>              return -1;
> @@ -453,6 +461,17 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
>                                     "direct-io-mode=1",
>                                     pool->def->target.path,
>                                     NULL);
> +    else if (cifsfs)
> +        cmd = virCommandNewArgList(MOUNT,
> +                                   "-t",
> +                                   (pool->def->type == VIR_STORAGE_POOL_FS ?

... so the first part of this ternary can't ever be true here.

> +                                    virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) :
> +                                    virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format)),
> +                                   src,
> +                                   pool->def->target.path,
> +                                   "-o",
> +                                   "guest",
> +                                   NULL);
>      else
>          cmd = virCommandNewArgList(MOUNT,
>                                     "-t",

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150615/e8000f0f/attachment-0001.sig>