diskbackend: Set relative path for media change

Via the virt-manager UI we aren't converting relative path to
absolute path, even though we do it internally when needed.

We were benefiting from this in the test suite in some ways, so we
need to adjust tests to strip out the dev dir on XML comparison

Signed-off-by: Cole Robinson <crobinso@redhat.com>
This commit is contained in:
Cole Robinson 2022-02-27 12:08:30 -05:00
parent 59595aebb7
commit 365d1f5d56
15 changed files with 67 additions and 22 deletions

View File

@ -15,7 +15,7 @@
<devices>
<disk type="file" device="disk">
<target dev="hda" bus="ide"/>
<source file="virt-install"/>
<source file="TESTSUITE_SCRUBBED/virt-install"/>
</disk>
<disk type="file" device="disk">
<target dev="hdb" bus="ide"/>

View File

@ -32,7 +32,7 @@
</disk>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-f26-netinst.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-f26-netinst.iso"/>
<target dev="sda" bus="scsi"/>
<readonly/>
</disk>

View File

@ -39,7 +39,7 @@
</disk>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-centos65-label.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-centos65-label.iso"/>
<target dev="hda" bus="ide"/>
<readonly/>
</disk>

View File

@ -39,7 +39,7 @@
</disk>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-fedora17-tree.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-fedora17-tree.iso"/>
<target dev="hda" bus="ide"/>
<readonly/>
</disk>
@ -115,6 +115,8 @@
<target dev="vda" bus="virtio"/>
</disk>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-fedora17-tree.iso"/>
<target dev="hda" bus="ide"/>
<readonly/>
</disk>

View File

@ -29,7 +29,7 @@
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-no-osinfo.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-no-osinfo.iso"/>
<target dev="hda" bus="ide"/>
<readonly/>
</disk>
@ -92,6 +92,8 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-no-osinfo.iso"/>
<target dev="hda" bus="ide"/>
<readonly/>
</disk>

View File

@ -34,7 +34,7 @@
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-f26-netinst.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-f26-netinst.iso"/>
<target dev="sda" bus="sata"/>
<readonly/>
</disk>
@ -102,6 +102,8 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-f26-netinst.iso"/>
<target dev="sda" bus="sata"/>
<readonly/>
</disk>

View File

@ -38,7 +38,7 @@
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-win7.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-win7.iso"/>
<target dev="sda" bus="sata"/>
<readonly/>
</disk>
@ -128,7 +128,7 @@
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="cdrom">
<driver name="qemu"/>
<source file="tests/data/fakemedia/fake-win7.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-win7.iso"/>
<target dev="sda" bus="sata"/>
<readonly/>
</disk>

View File

@ -17,7 +17,7 @@
<target dev="hdc" bus="ide"/>
<readonly/>
<address type="drive" controller="0" bus="1" target="0" unit="0"/>
<source file="tests/data/fakemedia/fake-win7.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-win7.iso"/>
</disk>
<controller type="ide" index="0"/>
<disk type="file" device="cdrom">
@ -51,7 +51,7 @@
<target dev="hdc" bus="ide"/>
<readonly/>
<address type="drive" controller="0" bus="1" target="0" unit="0"/>
<source file="tests/data/fakemedia/fake-win7.iso"/>
<source file="TESTSUITE_SCRUBBED/tests/data/fakemedia/fake-win7.iso"/>
</disk>
<controller type="ide" index="0"/>
</devices>

View File

@ -282,6 +282,11 @@ class Command(object):
if not output.endswith("\n"):
output += "\n"
# Strip the test directory out of the saved output
search = '"%s/' % utils.TOPDIR
if search in output:
output = output.replace(search, "\"TESTSUITE_SCRUBBED/")
utils.diff_compare(output, self.compare_file)
self.skip_checks.predefine_skip(conn)
@ -1521,7 +1526,7 @@ c.add_compare("--connect %(URI-TEST-FULL)s -o test-clone -n test --auto-clone --
c.add_valid(_CLONE_MANAGED + " --auto-clone --force-copy fda") # force copy empty floppy drive
c.add_invalid("-o idontexist --auto-clone", grep="Domain 'idontexist' was not found") # Non-existent vm name
c.add_invalid(_CLONE_UNMANAGED, grep="Either --auto-clone or --file") # XML file with several disks, but non specified
c.add_invalid(_CLONE_UNMANAGED + " --file virt-install", grep="overwrite the existing path 'virt-install'") # XML w/ disks, overwriting existing files with no --preserve
c.add_invalid(_CLONE_UNMANAGED + " --file virt-install", grep="overwrite the existing path") # XML w/ disks, overwriting existing files with no --preserve
c.add_invalid(_CLONE_MANAGED + " --file /tmp/clonevol", grep="matching name 'default-vol'") # will attempt to clone across pools, which test driver doesn't support
c.add_invalid(_CLONE_NOEXIST + " --auto-clone", grep="'/i/really/dont/exist' does not exist.") # XML w/ non-existent storage, WITHOUT --preserve

View File

@ -1154,3 +1154,21 @@ def testRefreshMachineType():
guest.os.machine = "s390-ccw-virtio-12345"
guest.refresh_machine_type()
assert guest.os.machine == "s390-ccw-virtio"
def testDiskSourceAbspath():
# If an existing disk doesn't have an abspath in the XML, make sure
# we don't convert it just by parsing
conn = utils.URIs.open_testdefault_cached()
xml = "<disk type='file' device='disk'><source file='foobar'/></disk>"
disk = virtinst.DeviceDisk(conn, parsexml=xml)
assert disk.get_source_path() == "foobar"
# But setting a relative path should convert it
import os
disk.set_source_path("foobar2")
assert disk.get_source_path() == os.path.abspath("foobar2")
# ...unless it's a URL
disk.set_source_path("http://example.com/foobar3")
assert disk.get_source_path() == "http://example.com/foobar3"

View File

@ -1,6 +1,9 @@
# This work is licensed under the GNU GPLv2 or later.
# See the COPYING file in the top-level directory.
import os
import tests
from . import lib
@ -10,6 +13,7 @@ from . import lib
def testMediaChange(app):
vmname = "test-many-devices"
app.uri = tests.utils.URIs.test_remote
app.open(show_console=vmname)
win = app.find_details_window(vmname,
click_details=True, shutdown=True)
@ -39,7 +43,7 @@ def testMediaChange(app):
entry.text == "Floppy_install_label (/dev/fdb)")
# Specify manual path
path = "/tmp/aaaaaaaaaaaaaaaaaaaaaaa.img"
path = "/pool-dir/UPPER"
entry.set_text(path)
appl.click()
lib.utils.check(lambda: not appl.sensitive)
@ -88,10 +92,16 @@ def testMediaHotplug(app):
entry = win.find("media-entry")
appl = win.find("config-apply")
# CDROM + physical
hw.find("IDE CDROM 1", "table cell").click()
lib.utils.check(lambda: not entry.text)
entry.set_text("/dev/sr0")
# Catch path does not exist error
entry.set_text("/dev/sr7")
appl.click()
app.click_alert_button("non-existent path '/dev/sr7", "Close")
# Check relative path while we are at it
path = "virt-install"
entry.set_text(path)
appl.click()
app.click_alert_button("changes will take effect", "OK")
lib.utils.check(lambda: not appl.sensitive)
@ -101,4 +111,4 @@ def testMediaHotplug(app):
win.find("Shut Down", "push button").click()
run = win.find("Run", "push button")
lib.utils.check(lambda: run.sensitive)
lib.utils.check(lambda: entry.text == "Fedora12_media (/dev/sr0)")
lib.utils.check(lambda: entry.text == os.path.abspath(path))

View File

@ -784,10 +784,12 @@ class vmmDomain(vmmLibvirtObject):
if not editdev:
return # pragma: no cover
validate = False
if path != _SENTINEL:
editdev.set_source_path(path)
if not do_hotplug:
editdev.sync_path_props()
validate = True
if readonly != _SENTINEL:
editdev.read_only = readonly
@ -806,6 +808,8 @@ class vmmDomain(vmmLibvirtObject):
if bus != _SENTINEL:
editdev.change_bus(self.xmlobj, bus)
if validate:
editdev.validate()
self._process_device_define(editdev, xmlobj, do_hotplug)
def define_network(self, devobj, do_hotplug,

View File

@ -610,7 +610,7 @@ class DeviceDisk(Device):
path = self._get_xmlpath()
if path and not vol_object and not parent_pool:
(vol_object, parent_pool) = diskbackend.manage_path(
(dummy, vol_object, parent_pool) = diskbackend.manage_path(
self.conn, path)
self._change_backend(path, vol_object, parent_pool)
@ -635,7 +635,8 @@ class DeviceDisk(Device):
# User explicitly changed 'path', so try to lookup its storage
# object since we may need it
(vol_object, parent_pool) = diskbackend.manage_path(self.conn, newpath)
(newpath, vol_object, parent_pool) = diskbackend.manage_path(
self.conn, newpath)
self._change_backend(newpath, vol_object, parent_pool)
self._set_xmlpath(self.get_source_path())

View File

@ -141,9 +141,9 @@ def manage_path(conn, path):
If path is not managed, try to create a storage pool to probe the path
"""
if not conn.support.conn_storage():
return None, None # pragma: no cover
return None, None, None # pragma: no cover
if not path:
return None, None
return None, None, None
if not path_is_url(path) and not path_is_network_vol(conn, path):
path = os.path.abspath(path)
@ -151,7 +151,7 @@ def manage_path(conn, path):
searchpath = _get_storage_search_path(path)
vol, pool = _check_if_path_managed(conn, searchpath)
if vol or pool or not _can_auto_manage(path):
return vol, pool
return path, vol, pool
dirname = os.path.dirname(path)
poolname = os.path.basename(dirname).replace(" ", "_")
@ -167,7 +167,7 @@ def manage_path(conn, path):
pool = poolxml.install(build=False, create=True, autostart=True)
vol = _lookup_vol_by_basename(pool, path)
return vol, pool
return path, vol, pool
def path_is_url(path):

View File

@ -595,7 +595,8 @@ class StorageVolume(_StorageObject):
log.debug("Attempting to detect format for backing_store=%s",
self.backing_store)
from . import diskbackend
vol, pool = diskbackend.manage_path(self.conn, self.backing_store)
path, vol, pool = diskbackend.manage_path(self.conn, self.backing_store)
dummy = path
if not vol: # pragma: no cover
log.debug("Didn't find any volume for backing_store")