Skip to content

Disk: add new dataStore element to disk.py#4052

Merged
Yingshun merged 1 commit intoavocado-framework:masterfrom
meinaLi:datastore
Mar 27, 2025
Merged

Disk: add new dataStore element to disk.py#4052
Yingshun merged 1 commit intoavocado-framework:masterfrom
meinaLi:datastore

Conversation

@meinaLi
Copy link
Copy Markdown
Contributor

@meinaLi meinaLi commented Jan 13, 2025

Now a new element dataStore has been implemented. So add it to disk.py.

@meinaLi meinaLi marked this pull request as draft January 13, 2025 06:32
@meinaLi
Copy link
Copy Markdown
Contributor Author

meinaLi commented Jan 13, 2025

>>>from virttest.libvirt_xml.devices import disk
>>>disk_dev = disk.Disk()
>>>disk_dev.xml = """
<disk type='file' device='disk'>
    <driver name='qemu' type='qcow2'/>
    <source file='/path/to/datastore.qcow2'>
      <dataStore type='file'>
        <format type='raw'/>
        <source file='/path/to/datastore'/>
      </dataStore>
    </source>
    <backingStore type='file'>
      <format type='qcow2'/>
      <source file='/var/lib/libvirt/images/base-with-data-file.qcow'>
        <dataStore type='block'>
          <format type='raw'/>
          <source dev='/dev/mapper/base2'/>
        </dataStore>
      </source>
    </backingStore>
    <target dev='vdh' bus='virtio'/>
  </disk>
  """
>>>disk_dev.fetch_attrs()
{'target': {'dev': 'vdh', 'bus': 'virtio'},
 'source': {'attrs': {'file': '/path/to/datastore.qcow2'},
  'dataStore': {'source': {'attrs': {'file': '/path/to/datastore'}},
   'type': 'file',
   'format': {'type': 'raw'}}},
 'driver': {'name': 'qemu', 'type': 'qcow2'},
 'device': 'disk',
 'type_name': 'file',
 'backingstore': {'source': {'datastore': {'source': {'attrs': {'dev': '/dev/mapper/base2'}},
    'type': 'block',
    'format': {'type': 'raw'}},
   'attrs': {'file': '/var/lib/libvirt/images/base-with-data-file.qcow'}},
  'type': 'file',
  'format': {'type': 'qcow2'}}}

@meinaLi meinaLi marked this pull request as ready for review January 14, 2025 07:01
@chloerh chloerh self-requested a review February 12, 2025 02:40
Comment thread virttest/libvirt_xml/devices/disk.py Outdated
Comment on lines +360 to +368
def new_datastore(self, **dargs):
"""
Return a new disk datastore instance and set properties from dargs
"""
new_one = self.dataStore(virsh_instance=self.virsh)
for key, value in list(dargs.items()):
setattr(new_one, key, value)
return new_one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is actually unnecessary now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Comment thread virttest/libvirt_xml/devices/disk.py Outdated
"""

__slots__ = ("attrs", "dev", "protocol", "name", "host", "file", "auth")
__slots__ = ("attrs", "protocol", "name", "host", "auth", "datastore")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems 'dev' was removed from slots , but not it's accessor.
And I'm also worried if it's going to cause error if we delete 'dev' and 'file', are they being used by some test code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't consider that other functions would use them. So in case I don't plan to delete them.

Comment thread virttest/libvirt_xml/devices/disk.py Outdated
Comment on lines +1036 to +1044
def new_source(self, **dargs):
"""
Create new source for dataStore

"""
new_one = self.Source(virsh_instance=self.virsh)
for key, value in list(dargs.items()):
setattr(new_one, key, value)
return new_one
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function could be removed too, we're using setup_attrs(), this kind of function is no longer necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@meinaLi meinaLi force-pushed the datastore branch 3 times, most recently from 1efbebe to 35be472 Compare February 19, 2025 06:47
@meinaLi meinaLi requested a review from chloerh February 19, 2025 08:04
Comment thread virttest/libvirt_xml/devices/disk.py Outdated
Comment on lines +238 to +245
accessors.XMLElementNest(
"datastore",
self,
parent_xpath="/",
tag_name="dataStore",
subclass=Disk.dataStore,
subclass_dargs={"virsh_instance": virsh_instance},
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meinaLi Can you plz provide a test result of this part? I didn't find it in your previous results.

BTW, for new changes in xml part, it's better to have a result of setup_attrs(). can you plz also provide it? Thanks!

Copy link
Copy Markdown
Contributor Author

@meinaLi meinaLi Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is in Disk class which is incorrect. Because dataStore element is mainly placed in the DiskSource or Backingchain. So I've already removed it.

The test result related to setup_attrs():

>>> from virttest.libvirt_xml.devices import disk
>>> disk_dev = disk.Disk()
>>> disk_dict={'device': 'disk', 'source': {'attrs': {'file': '/path/to/datastore.qcow2'}, 'dataStore': {'format': {'type': 'raw'}, 'type': 'file', 'source': {'attrs': {'file': '/path/to/datastore'}}}}, 'target': {'dev': 'vdh', 'bus': 'virtio'}, 'type_name': 'file', 'backingstore': {'type': 'file', 'format': {'type': 'qcow2'}, 'source': {'attrs': {'file': '/var/lib/libvirt/images/base-with-data-file.qcow'}, 'file': '/var/lib/libvirt/images/base-with-data-file.qcow', 'datastore': {'format': {'type': 'raw'}, 'type': 'block', 'source': {'attrs': {'dev': '/dev/mapper/base2'}}}}}, 'driver': {'name': 'qemu', 'type': 'qcow2'}}
>>> disk_dev.setup_attrs(**disk_dict)
>>> disk_dev.xml
'/tmp/xml_utils_temp_yhn4b_bv.xml'
# cat /tmp/xml_utils_temp_yhn4b_bv.xml
<disk type="file" device="disk"><source file="/path/to/datastore.qcow2"><dataStore type="file"><format type="raw" /><source file="/path/to/datastore" /></dataStore></source><target dev="vdh" bus="virtio" /><backingStore type="file"><format type="qcow2" /><source file="/var/lib/libvirt/images/base-with-data-file.qcow"><dataStore type="block"><format type="raw" /><source dev="/dev/mapper/base2" /></dataStore></source></backingStore><driver name="qemu" type="qcow2" /></disk>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the above disk_dict has a bug with XML error: The of can't have another nested or element. So here I mainly use the common disk xml test instead of the full disk xml.

Added the test result of attaching disk:

>>> from virttest.libvirt_xml.devices import disk
>>> disk_dev = disk.Disk()
>>> disk_dict={'device': 'disk', 'source': {'attrs': {'file': '/var/lib/libvirt/images/datastore.qcow2'}}, 'target': {'dev': 'vdh', 'bus': 'virtio'}, 'type_name': 'file', 'backingstore': {'type': 'file', 'format': {'type': 'qcow2'}, 'source': {'attrs': {'file': '/var/lib/libvirt/images/base-with-data-file.qcow'}, 'file': '/var/lib/libvirt/images/base-with-data-file.qcow', 'datastore': {'format': {'type': 'raw'}, 'type': 'file', 'source': {'attrs': {'file': '/var/lib/libvirt/images/datastore_1'}}}}}, 'driver': {'name': 'qemu', 'type': 'qcow2'}}
>>> disk_dev.setup_attrs(**disk_dict)
>>> from virttest import virsh
>>> virsh.attach_device("avocado-vt-vm1", disk_dev.xml, debug=True, ignore_status=False)
<avocado.utils.process.CmdResult object at 0x7f260c51da30>

The disk xml is attached to the guest:

# virsh dumpxml avocado-vt-vm1 --xpath //disk
......
<disk type="file" device="disk">
  <driver name="qemu" type="qcow2"/>
  <source file="/var/lib/libvirt/images/datastore.qcow2" index="2"/>
  <backingStore type="file" index="3">
    <format type="qcow2"/>
    <source file="/var/lib/libvirt/images/base-with-data-file.qcow">
      <dataStore type="file">
        <format type="raw"/>
        <source file="/var/lib/libvirt/images/datastore_1" index="4"/>
      </dataStore>
    </source>
    <backingStore/>
  </backingStore>
  <target dev="vdh" bus="virtio"/>
  <alias name="virtio-disk7"/>
  <address type="pci" domain="0x0000" bus="0x05" slot="0x00" function="0x0"/>
</disk>

Now a new element dataStore has been implemented. So add it to disk.py.

Signed-off-by: meinaLi <meili@redhat.com>
@Yingshun Yingshun merged commit 978c026 into avocado-framework:master Mar 27, 2025
50 checks passed
@meinaLi meinaLi deleted the datastore branch June 26, 2025 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants