Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for snapshot disk source #3906

Conversation

nanli1
Copy link
Contributor

@nanli1 nanli1 commented Apr 25, 2024

Need disk source in snapshot xml
Signed-off-by: nanli nanli@redhat.com
For autotest/tp-libvirt#5584

   Need disk source in snapshot xml
Signed-off-by: nanli <nanli@redhat.com>
Copy link
Contributor

@cliping cliping left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@chloerh chloerh left a comment

Choose a reason for hiding this comment

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

Seems Disk class already has 'source', is it possible that we don't need to add it again?

@nanli1
Copy link
Contributor Author

nanli1 commented May 8, 2024

Seems Disk class already has 'source', is it possible that we don't need to add it again?

hi haijiao ,Thanks your comment .This is for disk > source tag is in snapshot xml instead of disk xml ,Did I get your point

# cat s1.xml
<domainsnapshot>
<description>Snapshot test</description>
<name>s1</name>
<memory snapshot='no'/>
<disks>
<disk name='vda' snapshot='no'/>
<disk name='vdb' snapshot='external'>
<source file="/var/lib/libvirt/images/vdb.s1"/>
</disk>
</disks>
</domainsnapshot>

@nanli1 nanli1 requested a review from chloerh May 9, 2024 02:13
@nanli1 nanli1 closed this May 9, 2024
@nanli1
Copy link
Contributor Author

nanli1 commented May 9, 2024

Haijiao has helped to check that we have inherit disk class , no need to give this support

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