Bug 1208763 - virsh pool-create-as cannot ether recognise nor handle relative path
Summary: virsh pool-create-as cannot ether recognise nor handle relative path
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.1
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Erik Skultety
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-04-03 06:32 UTC by Xichen Zhou
Modified: 2015-11-19 06:27 UTC (History)
6 users (show)

Fixed In Version: libvirt-1.2.15-1.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-19 06:27:12 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1021000 0 unspecified CLOSED virsh: filesystem paths are not taken relative to cwd 2021-02-22 00:41:40 UTC
Red Hat Product Errata RHBA-2015:2202 0 normal SHIPPED_LIVE libvirt bug fix and enhancement update 2015-11-19 08:17:58 UTC

Internal Links: 1021000

Description Xichen Zhou 2015-04-03 06:32:06 UTC
description of the problem:
virsh pool-create-as cannot ether recognise nor handle relative path

Version-Release number of selected component (if applicable):
libvirt-1.2.8-16.el7.x86_64

How reproducible:
100%


Steps to reproduce:

1. Prepare the libvirt environment

#pwd
/root
#mkdir -p ./virshpool

#ls virtpool

2. create pool with relative path

#virsh pool-create-as test1 dir --target ./virtpool
error: Failed to create pool test1
error: cannot open path './virtpool': No such file or directory

#virsh pool-create-as test1 dir --target ../tmp
Pool test1 created

#virsh pool-dumpxml test1

<pool type='dir'>
  <name>test1</name>
  <uuid>f16ba91f-a91f-3dd1-829f-f53c1a6d2183</uuid>
  <capacity unit='bytes'>52710469632</capacity>
  <allocation unit='bytes'>29068619776</allocation>
  <available unit='bytes'>23641849856</available>
  <source>
  </source>
  <target>
    <path>../tmp</path>
    <permissions>
      <mode>0755</mode>
      <owner>-1</owner>
      <group>-1</group>
    </permissions>
  </target>
</pool>


Actual result:
pool was created at somewhere we don't know,
there shouldn't be any relative path in the pool's configure xml.

Expect result:
Failed to create.

3. This problem is related to environment variable of libvirtd, 
in which its ${PWD} is /. You can do even wireder things like this.

#mkdir -p /test3
#ll ./test3
ll: no such file or directory

#virsh pool-create-as test3 dir --target ./test3
Pool test3 created

#virsh pool-dumpxml test3
<pool type='dir'>
  <name>test3</name>
  <uuid>4b9b0202-89a8-4769-833e-565daa5bbc3a</uuid>
  <capacity unit='bytes'>53660876800</capacity>
  <allocation unit='bytes'>3289653248</allocation>
  <available unit='bytes'>50371223552</available>
  <source>
  </source>
  <target>
    <path>./test3</path>
    <permissions>
      <mode>0755</mode>
      <owner>-1</owner>
      <group>-1</group>
    </permissions>
  </target>
</pool>

Actual result:
In virt-manage, obviously you cannot use this pool to create a guest.

Expect result:
This pool should not be created.

Additional Information:

Comment 2 Erik Skultety 2015-04-07 12:12:06 UTC
You would get the same message "No such file or directory" if you used a relative path in any other <source> or <target> element (e.g. disk source) which in my opinion provides pretty accurate explanation of what's going on. As you very correctly noted, it's because libvirtd is runnig with CWD '/' so anything relative to '/' would work, like this one:

> #virsh pool-create-as test1 dir --target ../tmp
> Pool test1 created
 
The only thing is, we don't have this explicitly documented with storage pools (however we do have it for storage volumes), so I proposed a patch to update the docs: https://www.redhat.com/archives/libvir-list/2015-April/msg00209.html

Comment 3 Xichen Zhou 2015-04-08 01:55:25 UTC
(In reply to Erik Skultety from comment #2)
> You would get the same message "No such file or directory" if you used a
> relative path in any other <source> or <target> element (e.g. disk source)
> which in my opinion provides pretty accurate explanation of what's going on.
> As you very correctly noted, it's because libvirtd is runnig with CWD '/' so
> anything relative to '/' would work, like this one:
> 
> > #virsh pool-create-as test1 dir --target ../tmp
> > Pool test1 created
>
But it is so confusing for anyone who doesn't know about it, plus it is against the user interface of a unix program.

This problem can be easily tackled in virsh interface, just change the path to absolute path according to $PWD of virsh command. But there is so many wrappers for libvirt, developers like you may decided to pass any argument directly to libvirtd. Thus there has to be checking mechanism.
 
> The only thing is, we don't have this explicitly documented with storage
> pools (however we do have it for storage volumes), so I proposed a patch to
> update the docs:
> https://www.redhat.com/archives/libvir-list/2015-April/msg00209.html

Comment 4 Erik Skultety 2015-04-09 12:05:43 UTC
virsh can be used to manage remote machines as well. This means, handling paths according to virsh client CWD is definitely not an option.
I can't see anything confusing, almost any unix program returns system errors when dealing with filesystems (this includes "no such file or directory"), that means there is a checking mechanism, however ignoring the error completely or letting the daemon crash would be a completely different story, with that I agree...

Fixed upstream:

commit 3888dcaa670d811cee05b16c8b3ddf89a4273959
Author: Erik Skultety <eskultet@redhat.com>
Date:   Tue Apr 7 12:46:13 2015 +0200

    doc: Add info (where necessary) that paths should be specified as absolute
    
    We documented this almost everywhere, but missed it on several places.

v1.2.14-74-g3888dca

Comment 6 yisun 2015-06-03 08:55:26 UTC
Hi Erik,
The relative path in pool creation surely cause confusion.
And it will cause potential problems if several different pools pointing to same dir. 
As CWD pointing to "/":
../../tmp
./tmp
../tmp
will all pointing to /tmp
And creating pools with these source paths will all succeed. And corresponding volume operations in one pool will surely affect other pools. 

Libvirt forbids using single source to create different pools and it'll generate error as follow :
# virsh pool-create-as test6 dir --target ../tmp
error: Failed to create pool test6
error: operation failed: Storage source conflict with pool: 'test5'   <=== test5 also use "../tmp" as source

As you said:
============================
virsh can be used to manage remote machines as well. This means, handling paths according to virsh client CWD is definitely not an option.
============================

But is it possible to forbid the relative path usage only when pool_type=dir ?

Comment 7 Erik Skultety 2015-06-03 14:35:30 UTC
We've already documented that we expect absolute paths only. From my point of view, the best place to check for this is in the post-parse phase (definitely not special-casing/limiting this to pool-type 'dir' only).
However, as I said here  https://bugzilla.redhat.com/show_bug.cgi?id=1021000, if someone does have a functional machine using relative paths in their XML (very unfortunate), they'd loose the machine after daemon restart and from libvirt (as a product) point of view, we surely don't want such a thing unless it would cause a critical error. I think it should be the user's responsibility to expect the consequences if they play with relative paths the way you showed in your example above.
One more thing, I was just looking at the RNG schemas and I found that we have a rule that requires the path to be absolute, so the ideal use case should be:
write an XML definition -> RNG validator checks the XML -> user passes the XML to the client and client handles it

However, not everything can be checked by RNG validator, there are some logic related scenarios which have to be handled during runtime, that's where post-parse phase comes in and even then there might be issues like: user provided a valid emulator (RNG schema PASSED), also they provided right values for all the attributes (XML post-parse phase PASSED), but they don't have the required version to support all the flags and parameters for the emulator! (this is checked right before we execute the binary)

Comment 9 yisun 2015-06-05 09:16:34 UTC
Verified on libvirt.org

http://libvirt.org/formatdomain.html

loader
    The optional loader tag refers to a firmware blob, which is specified by absolute path, used to assist the domain creation process. It is used by Xen fully virtualized domains as well as setting the QEMU BIOS file path for QEMU/KVM domains.

...

nvram
    Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the absolute path to the file is stored in this element.
    ...
    Resource partitioning
    Hypervisors may allow for virtual machines to be placed into resource partitions, potentially with nesting of said partitions. The resource element groups together configuration related to resource partitioning. It currently supports a child element partition whose content defines the absolute path of the resource partition in which to place the domain.
    ...

    type='block' since 0.0.3
        The dev attribute specifies the fully-qualified path to the host device to serve as the disk.
        ...

Since 0.9.7 (QEMU and KVM only). The optional file attribute contains an absolute path to a binary file to be presented to the guest as the device's ROM BIOS. This can be useful, for example, to provide a PXE boot ROM for a virtual function of an sr-iov capable ethernet device (which has no boot ROMs for the VFs). Since 0.9.10 (QEMU and KVM only).
...
Block / character devices
Block / character devices from the host can be passed through to the guest using the hostdev element. This is only possible with container based virtualization. Devices are specified by a fully qualified path.
...

master
    Master device of the pair, that is passed to the hypervisor. Device is specified by a fully qualified path.

    slave
        Slave device of the pair, that is passed to the clients for connection to the guest console. Device is specified by a fully qualified path.

    ...
    This backend type requires exclusive access to a TPM device on the host. An example for such a device is /dev/tpm0. The fully qualified file name is specified by path attribute of the source element. If no file name is specified then /dev/tpm0 is automatically used. 
    ...

    server
        The optional server element can be used to configure a server socket the device is supposed to connect to. The optional path attribute specifies the absolute path to the unix socket and defaults to /var/lib/libvirt/shmem/$shmem-$name-sock.
        ...

http://libvirt.org/formatstorage.html

path
    Provides the location at which the pool will be mapped into the local filesystem namespace, as an absolute path. For a filesystem/directory based pool it will be a fully qualified name of the directory in which volumes will be created. For device based pools it will be a fully qualified name of the directory in which devices nodes exist. For the latter /dev/ may seem like the logical choice, however, devices nodes there are not guaranteed stable across reboots, since they are allocated on demand. It is preferable to use a stable location such as one of the /dev/disk/by-{path|id|uuid|label} locations. Since 0.4.1

Comment 11 errata-xmlrpc 2015-11-19 06:27:12 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-2202.html


Note You need to log in before you can comment on or make changes to this bug.