Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Christophe de Dinechin]

> On 17 Oct 2019, at 14:35, Cole Robinson  wrote:
> 
> Hmm why wasn't I in to/cc list, was that intentional?

Not at all. I assumed a reply-to would add it automatically and did not
bother checking. Probably something wrong in my mu4e config.

> 
> On 10/17/19 5:26 AM, Christophe de Dinechin wrote:
>> 
>> Cole Robinson writes:
>> 
>>> This series is the first steps to teaching libvirt about qcow2
>>> data_file support, aka external data files or qcow2 external metadata.
>>> 
>>> A bit about the feature: it was added in qemu 4.0. It essentially
>>> creates a two part image file: a qcow2 layer that just tracks the
>>> image metadata, and a separate data file which is stores the VM
>> 
>> s/is stores/stores/
> 
> Indeed, but what's the benefit of correcting grammar in a cover letter
> for already applied patches?

Obviously, I realized you had already applied that patch only after sending
my email ;-) I saw that there was still some active discussion on that thread,
so I assumed it was still under review.

>> 
>> What happens if you try to do that in place, i.e. EXISTING==NEW?
>> 
> 
> Results in an unbootable image in my testing. I'm guessing it
> initializes the raw image first, similar to trying to use 'qemu-img
> create' with an existing raw image as data_file
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1688814#c8 
> 

OK. Might be worth catching or fixing at some point.


Christophe--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

Hmm why wasn't I in to/cc list, was that intentional?

On 10/17/19 5:26 AM, Christophe de Dinechin wrote:
> 
> Cole Robinson writes:
> 
>> This series is the first steps to teaching libvirt about qcow2
>> data_file support, aka external data files or qcow2 external metadata.
>>
>> A bit about the feature: it was added in qemu 4.0. It essentially
>> creates a two part image file: a qcow2 layer that just tracks the
>> image metadata, and a separate data file which is stores the VM
> 
> s/is stores/stores/

Indeed, but what's the benefit of correcting grammar in a cover letter
for already applied patches?

> 
>> disk contents. AFAICT the driving use case is to keep a fully coherent
>> raw disk image on disk, and only use qcow2 as an intermediate metadata
>> layer when necessary, for things like incremental backup support.
>>
>> The original qemu patch posting is here:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html
>>
>> For testing, you can create a new qcow2+raw data_file image from an
>> existing image, like:
>>
>> qemu-img convert -O qcow2 \
>> -o data_file=NEW.raw,data_file_raw=yes
>> EXISTING.raw NEW.qcow2
> 
> What happens if you try to do that in place, i.e. EXISTING==NEW?
> 

Results in an unbootable image in my testing. I'm guessing it
initializes the raw image first, similar to trying to use 'qemu-img
create' with an existing raw image as data_file

https://bugzilla.redhat.com/show_bug.cgi?id=1688814#c8

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Christophe de Dinechin]

Cole Robinson writes:

> This series is the first steps to teaching libvirt about qcow2
> data_file support, aka external data files or qcow2 external metadata.
>
> A bit about the feature: it was added in qemu 4.0. It essentially
> creates a two part image file: a qcow2 layer that just tracks the
> image metadata, and a separate data file which is stores the VM

s/is stores/stores/

> disk contents. AFAICT the driving use case is to keep a fully coherent
> raw disk image on disk, and only use qcow2 as an intermediate metadata
> layer when necessary, for things like incremental backup support.
>
> The original qemu patch posting is here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html
>
> For testing, you can create a new qcow2+raw data_file image from an
> existing image, like:
>
> qemu-img convert -O qcow2 \
> -o data_file=NEW.raw,data_file_raw=yes
> EXISTING.raw NEW.qcow2

What happens if you try to do that in place, i.e. EXISTING==NEW?

>
> The goal of this series is to teach libvirt enough about this case
> so that we can correctly relabel the data_file on VM startup/shutdown.
> The main functional changes are
>
>   * Teach storagefile how to parse out data_file from the qcow2 header
>   * Store the raw string as virStorageSource->externalDataStoreRaw
>   * Track that as its out virStorageSource in externalDataStore
>   * dac/selinux relabel externalDataStore as needed
>

--
Cheers,
Christophe de Dinechin (IRC c3d)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Han Han]

On Wed, Oct 16, 2019 at 1:04 AM Cole Robinson  wrote:

> On 10/15/19 3:56 AM, Han Han wrote:
> > Hello Cole, one issue is found:
> > The qcow2 data file XTTRs is not cleaned on external snapshot when
> > -blockdev is not enabled
> >
> > Versions:
> > libvirt v5.8.0-134-g9d03e9adf1
> > qemu-kvm-4.1.0-13.module+el8.1.0+4313+ef76ec61.x86_64
> >
> > Steps:
> > 1. Convert a OS image to qcow2 data file:
> > # qemu-img convert -O qcow2 -o
> > data_file=/var/lib/libvirt/images/pc-data.raw,data_file_raw=on
> > /var/lib/libvirt/images/pc.qcow2 /var/lib/libvirt/images/pc-data.qcow2
> >
> > 2. Build and start libvirt source, start libvirt daemon:
> > # make clean && CC=/usr/lib64/ccache/cc ./autogen.sh&&./configure
> > --without-libssh --build=x86_64-redhat-linux-gnu
> > --host=x86_64-redhat-linux-gnu --program-prefix=
> > --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
> > --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
> > --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
> > --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
> > --mandir=/usr/share/man --infodir=/usr/share/info --with-qemu
> > --without-openvz --without-lxc --without-vbox --without-libxl
> > --with-sasl --with-polkit --with-libvirtd --without-phyp --with-esx
> > --without-hyperv --without-vmware --without-xenapi --without-vz
> > --without-bhyve --with-interface --with-network --with-storage-fs
> > --with-storage-lvm --with-storage-iscsi --with-storage-iscsi-direct
> > --with-storage-scsi --with-storage-disk --with-storage-mpath
> > --with-storage-rbd --without-storage-sheepdog --with-storage-gluster
> > --without-storage-zfs --without-storage-vstorage --with-numactl
> > --with-numad --with-capng --without-fuse --with-netcf --with-selinux
> > --with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal
> > --with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap
> > --with-audit --with-dtrace --with-driver-modules --with-firewalld
> > --with-firewalld-zone --without-wireshark-dissector --without-pm-utils
> > --with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01,
> > lab.rhel8.me ' --with-packager-version=1.el8
> > --with-qemu-user=qemu --with-qemu-group=qemu
> > --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror
> > --enable-expensive-tests --with-init-script=systemd
> > --without-login-shell && make -j8
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/virtlogd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" LIBVIRT_DEBUG=3
> > LIBVIRT_LOG_FILTERS="1:util 1:qemu 1:security"
> > LIBVIRT_LOG_OUTPUTS="1:file:/tmp/libvirt_daemon.log" src/.libs/libvirtd
> >
> > 3. Define and start an VM with the qcow2 data file. Note that the
> > -blockdev is not enabled
> > # virsh define pc-data.xml
> > # virsh start pc-data
> >
> > 4. Create snapshot and check the data file XATTRs:
> > # virsh snapshot-create-as pc-data s1 --no-metadata --disk-only
> > # getfattr -m - -d /var/lib/libvirt/images/pc-data.raw
> > getfattr: Removing leading '/' from absolute path names
> > # file: var/lib/libvirt/images/pc-data.raw
> > security.selinux="unconfined_u:object_r:svirt_image_t:s0:c775,c1011"
> > trusted.libvirt.security.dac="+107:+107"
> > trusted.libvirt.security.ref_dac="1"
> > trusted.libvirt.security.ref_selinux="1"
> >
> trusted.libvirt.security.selinux="unconfined_u:object_r:svirt_image_t:s0:c284,c367"
> > trusted.libvirt.security.timestamp_dac="1563328069"
> > trusted.libvirt.security.timestamp_selinux="1563328069"
> >
> > Shutdown the VM. The XATTRs of data file is not changed.
> > It is not expected. The XTTRs should not contain *.libvirt.*
> >
> > Issue is not reproduced with -blockdev enabled:
> > 
> > ...
> >   
> > 
> > 
> >   
> > 
> >
> > See the libvirt daemon log and vm xml in attachment.
>
> Nice catch! I will need to dig into this to figure out where the issue
> is. Can you put this info into an upstream bug report in
>
Sure. https://bugzilla.redhat.com/show_bug.cgi?id=1762135

> product=Virtualization Tools  and I will get to it when I can
>
> Thanks,
> Cole
>


-- 
Best regards,
---
Han Han
Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/15/19 3:56 AM, Han Han wrote:
> Hello Cole, one issue is found:
> The qcow2 data file XTTRs is not cleaned on external snapshot when
> -blockdev is not enabled
> 
> Versions:
> libvirt v5.8.0-134-g9d03e9adf1
> qemu-kvm-4.1.0-13.module+el8.1.0+4313+ef76ec61.x86_64
> 
> Steps:
> 1. Convert a OS image to qcow2 data file:
> # qemu-img convert -O qcow2 -o
> data_file=/var/lib/libvirt/images/pc-data.raw,data_file_raw=on
> /var/lib/libvirt/images/pc.qcow2 /var/lib/libvirt/images/pc-data.qcow2
> 
> 2. Build and start libvirt source, start libvirt daemon:
> # make clean && CC=/usr/lib64/ccache/cc ./autogen.sh&&./configure
> --without-libssh --build=x86_64-redhat-linux-gnu
> --host=x86_64-redhat-linux-gnu --program-prefix=
> --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
> --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
> --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
> --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
> --mandir=/usr/share/man --infodir=/usr/share/info --with-qemu
> --without-openvz --without-lxc --without-vbox --without-libxl
> --with-sasl --with-polkit --with-libvirtd --without-phyp --with-esx
> --without-hyperv --without-vmware --without-xenapi --without-vz
> --without-bhyve --with-interface --with-network --with-storage-fs
> --with-storage-lvm --with-storage-iscsi --with-storage-iscsi-direct
> --with-storage-scsi --with-storage-disk --with-storage-mpath
> --with-storage-rbd --without-storage-sheepdog --with-storage-gluster
> --without-storage-zfs --without-storage-vstorage --with-numactl
> --with-numad --with-capng --without-fuse --with-netcf --with-selinux
> --with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal
> --with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap
> --with-audit --with-dtrace --with-driver-modules --with-firewalld
> --with-firewalld-zone --without-wireshark-dissector --without-pm-utils
> --with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01,
> lab.rhel8.me ' --with-packager-version=1.el8
> --with-qemu-user=qemu --with-qemu-group=qemu
> --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror
> --enable-expensive-tests --with-init-script=systemd
> --without-login-shell && make -j8
> # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/virtlogd
> # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" LIBVIRT_DEBUG=3
> LIBVIRT_LOG_FILTERS="1:util 1:qemu 1:security"
> LIBVIRT_LOG_OUTPUTS="1:file:/tmp/libvirt_daemon.log" src/.libs/libvirtd
> 
> 3. Define and start an VM with the qcow2 data file. Note that the
> -blockdev is not enabled
> # virsh define pc-data.xml
> # virsh start pc-data
> 
> 4. Create snapshot and check the data file XATTRs:
> # virsh snapshot-create-as pc-data s1 --no-metadata --disk-only
> # getfattr -m - -d /var/lib/libvirt/images/pc-data.raw
> getfattr: Removing leading '/' from absolute path names
> # file: var/lib/libvirt/images/pc-data.raw
> security.selinux="unconfined_u:object_r:svirt_image_t:s0:c775,c1011"
> trusted.libvirt.security.dac="+107:+107"
> trusted.libvirt.security.ref_dac="1"
> trusted.libvirt.security.ref_selinux="1"
> trusted.libvirt.security.selinux="unconfined_u:object_r:svirt_image_t:s0:c284,c367"
> trusted.libvirt.security.timestamp_dac="1563328069"
> trusted.libvirt.security.timestamp_selinux="1563328069"
> 
> Shutdown the VM. The XATTRs of data file is not changed.
> It is not expected. The XTTRs should not contain *.libvirt.*
> 
> Issue is not reproduced with -blockdev enabled:
> 
> ...
>   
>     
>     
>   
> 
> 
> See the libvirt daemon log and vm xml in attachment.

Nice catch! I will need to dig into this to figure out where the issue
is. Can you put this info into an upstream bug report in
product=Virtualization Tools  and I will get to it when I can

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/15/19 12:29 AM, Han Han wrote:
> I find the issue cannot reproduced when `make clean` before build the
> source.
> It is not proper to build with an unclean source dir, right?
> 

I don't use 'make clean' in libvirt.git but in other projects I have hit
issues that required 'make clean', like in qemu if trying to run old
commits, often the build system gets confused. I dip in and out of
libvirt development though so maybe regular devs have hit similar issues

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Han Han]

I find the issue cannot reproduced when `make clean` before build the
source.
It is not proper to build with an unclean source dir, right?

On Tue, Oct 15, 2019 at 3:55 AM Cole Robinson  wrote:

> On 10/12/19 11:07 AM, Han Han wrote:
> >
> >
> > On Sat, Oct 12, 2019 at 1:05 AM Cole Robinson  > > wrote:
> >
> > On 10/10/19 11:25 PM, Han Han wrote:
> > > Hi Cole,
> > > I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new
> > > capabilities introduced by these to branches to resolve conflicts.
> > > Then build and test as following:
> > > # ./autogen.sh&& ./configure --without-libssh
> > > --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu
> > > --program-prefix= --disable-dependency-tracking --prefix=/usr
> > > --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
> > > --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include
> > > --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var
> > > --sharedstatedir=/var/lib --mandir=/usr/share/man
> > > --infodir=/usr/share/info --with-qemu --without-openvz
> --without-lxc
> > > --without-vbox --without-libxl --with-sasl --with-polkit
> > --with-libvirtd
> > > --without-phyp --with-esx --without-hyperv --without-vmware
> > > --without-xenapi --without-vz --without-bhyve --with-interface
> > > --with-network --with-storage-fs --with-storage-lvm
> > --with-storage-iscsi
> > > --with-storage-iscsi-direct --with-storage-scsi --with-storage-disk
> > > --with-storage-mpath --with-storage-rbd --without-storage-sheepdog
> > > --with-storage-gluster --without-storage-zfs
> > --without-storage-vstorage
> > > --with-numactl --with-numad --with-capng --without-fuse
> --with-netcf
> > > --with-selinux --with-selinux-mount=/sys/fs/selinux
> > --without-apparmor
> > > --without-hal --with-udev --with-yajl --with-sanlock --with-libpcap
> > > --with-macvtap --with-audit --with-dtrace --with-driver-modules
> > > --with-firewalld --with-firewalld-zone
> --without-wireshark-dissector
> > > --without-pm-utils --with-nss-plugin '--with-packager=Unknown,
> > > 2019-08-19-12:13:01, lab.rhel8.me 
> > '
> > > --with-packager-version=1.el8 --with-qemu-user=qemu
> > > --with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM
> > > --enable-werror --enable-expensive-tests --with-init-script=systemd
> > > --without-login-shell && make
> > >
> > > Start libvirtd and virtlogd
> > > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')"
> > src/.libs/libvirtd
> > > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')"
> ./src/virtlogd
> > >
> > > Then try to list all domains:
> > > # virsh list --all
> > >
> > > Libvirtd exits with segment fault:
> > > [1]30104 segmentation fault (core dumped)  LD_PRELOAD="$(find
> src
> > > -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
> > >
> > > Version:
> > > qemu-4.1
> > >
> > > Backtrace:
> > > (gdb) bt
> > > #0  0x7fbe57a0d1b9 in
> virDomainVirtioSerialAddrSetAddControllers
> > > (def=, def=, addrs=)
> at
> > > conf/domain_addr.c:1656
> > > #1  virDomainVirtioSerialAddrSetCreateFromDomain
> > > (def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753
> > > #2  0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses
> > > (def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174
> > > #3  qemuDomainAssignAddresses (def=0x7fbde81cc3f0,
> > > qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0,
> > > newDomain=) at qemu/qemu_domain_address.c:3174
> > > #4  0x7fbe57a39e0d in virDomainDefPostParse
> > > (def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20,
> > > parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry
> =0x7fbde83ce070,
> > >  parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858
> > > #5  0x7fbe57a525c5 in virDomainDefParseNode (xml= out>,
> > > root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070,
> > > parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677
> > > #6  0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry
> =0x0,
> > > filename=, caps=caps@entry=0x7fbde8154d20,
> > > xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry
> =0x0,
> > >  flags=flags@entry=4610) at conf/domain_conf.c:21628
> > > #7  0x7fbe57a528f6 in virDomainDefParseFile
> (filename= > > out>, caps=caps@entry=0x7fbde8154d20,
> > > xmlopt=xmlopt@entry=0x7fbde83ce070,
> > parseOpaque=parseOpaque@entry=0x0,
> > > flags=flags@entry=4610)
> > >  at conf/domain_conf.c:21653
> > > #8  0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0,
> > > notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070
> > > 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/12/19 11:07 AM, Han Han wrote:
> 
> 
> On Sat, Oct 12, 2019 at 1:05 AM Cole Robinson  > wrote:
> 
> On 10/10/19 11:25 PM, Han Han wrote:
> > Hi Cole,
> > I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new
> > capabilities introduced by these to branches to resolve conflicts.
> > Then build and test as following:
> > # ./autogen.sh&& ./configure --without-libssh
> > --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu
> > --program-prefix= --disable-dependency-tracking --prefix=/usr
> > --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
> > --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include
> > --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var
> > --sharedstatedir=/var/lib --mandir=/usr/share/man
> > --infodir=/usr/share/info --with-qemu --without-openvz --without-lxc
> > --without-vbox --without-libxl --with-sasl --with-polkit
> --with-libvirtd
> > --without-phyp --with-esx --without-hyperv --without-vmware
> > --without-xenapi --without-vz --without-bhyve --with-interface
> > --with-network --with-storage-fs --with-storage-lvm
> --with-storage-iscsi
> > --with-storage-iscsi-direct --with-storage-scsi --with-storage-disk
> > --with-storage-mpath --with-storage-rbd --without-storage-sheepdog
> > --with-storage-gluster --without-storage-zfs
> --without-storage-vstorage
> > --with-numactl --with-numad --with-capng --without-fuse --with-netcf
> > --with-selinux --with-selinux-mount=/sys/fs/selinux
> --without-apparmor
> > --without-hal --with-udev --with-yajl --with-sanlock --with-libpcap
> > --with-macvtap --with-audit --with-dtrace --with-driver-modules
> > --with-firewalld --with-firewalld-zone --without-wireshark-dissector
> > --without-pm-utils --with-nss-plugin '--with-packager=Unknown,
> > 2019-08-19-12:13:01, lab.rhel8.me 
> '
> > --with-packager-version=1.el8 --with-qemu-user=qemu
> > --with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM
> > --enable-werror --enable-expensive-tests --with-init-script=systemd
> > --without-login-shell && make
> >
> > Start libvirtd and virtlogd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')"
> src/.libs/libvirtd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd
> >
> > Then try to list all domains:
> > # virsh list --all
> >
> > Libvirtd exits with segment fault:
> > [1]    30104 segmentation fault (core dumped)  LD_PRELOAD="$(find src
> > -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
> >
> > Version:
> > qemu-4.1
> >
> > Backtrace:
> > (gdb) bt
> > #0  0x7fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers
> > (def=, def=, addrs=) at
> > conf/domain_addr.c:1656
> > #1  virDomainVirtioSerialAddrSetCreateFromDomain
> > (def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753
> > #2  0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses
> > (def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174
> > #3  qemuDomainAssignAddresses (def=0x7fbde81cc3f0,
> > qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0,
> > newDomain=) at qemu/qemu_domain_address.c:3174
> > #4  0x7fbe57a39e0d in virDomainDefPostParse
> > (def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20,
> > parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry=0x7fbde83ce070,
> >      parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858
> > #5  0x7fbe57a525c5 in virDomainDefParseNode (xml=,
> > root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070,
> > parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677
> > #6  0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0,
> > filename=, caps=caps@entry=0x7fbde8154d20,
> > xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0,
> >      flags=flags@entry=4610) at conf/domain_conf.c:21628
> > #7  0x7fbe57a528f6 in virDomainDefParseFile (filename= > out>, caps=caps@entry=0x7fbde8154d20,
> > xmlopt=xmlopt@entry=0x7fbde83ce070,
> parseOpaque=parseOpaque@entry=0x0,
> > flags=flags@entry=4610)
> >      at conf/domain_conf.c:21653
> > #8  0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0,
> > notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070
> > "/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050
> > "/etc/libvirt/qemu",
> >      xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20,
> doms=0x7fbde8126940) at
> > conf/virdomainobjlist.c:503
> > #9  virDomainObjListLoadAllConfigs (doms=0x7fbde8126940,
> > configDir=0x7fbde8124050 "/etc/libvirt/qemu",
> > autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart",
> > 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Han Han]

On Sat, Oct 12, 2019 at 1:05 AM Cole Robinson  wrote:

> On 10/10/19 11:25 PM, Han Han wrote:
> > Hi Cole,
> > I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new
> > capabilities introduced by these to branches to resolve conflicts.
> > Then build and test as following:
> > # ./autogen.sh&& ./configure --without-libssh
> > --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu
> > --program-prefix= --disable-dependency-tracking --prefix=/usr
> > --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
> > --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include
> > --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var
> > --sharedstatedir=/var/lib --mandir=/usr/share/man
> > --infodir=/usr/share/info --with-qemu --without-openvz --without-lxc
> > --without-vbox --without-libxl --with-sasl --with-polkit --with-libvirtd
> > --without-phyp --with-esx --without-hyperv --without-vmware
> > --without-xenapi --without-vz --without-bhyve --with-interface
> > --with-network --with-storage-fs --with-storage-lvm --with-storage-iscsi
> > --with-storage-iscsi-direct --with-storage-scsi --with-storage-disk
> > --with-storage-mpath --with-storage-rbd --without-storage-sheepdog
> > --with-storage-gluster --without-storage-zfs --without-storage-vstorage
> > --with-numactl --with-numad --with-capng --without-fuse --with-netcf
> > --with-selinux --with-selinux-mount=/sys/fs/selinux --without-apparmor
> > --without-hal --with-udev --with-yajl --with-sanlock --with-libpcap
> > --with-macvtap --with-audit --with-dtrace --with-driver-modules
> > --with-firewalld --with-firewalld-zone --without-wireshark-dissector
> > --without-pm-utils --with-nss-plugin '--with-packager=Unknown,
> > 2019-08-19-12:13:01, lab.rhel8.me '
> > --with-packager-version=1.el8 --with-qemu-user=qemu
> > --with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM
> > --enable-werror --enable-expensive-tests --with-init-script=systemd
> > --without-login-shell && make
> >
> > Start libvirtd and virtlogd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd
> >
> > Then try to list all domains:
> > # virsh list --all
> >
> > Libvirtd exits with segment fault:
> > [1]30104 segmentation fault (core dumped)  LD_PRELOAD="$(find src
> > -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
> >
> > Version:
> > qemu-4.1
> >
> > Backtrace:
> > (gdb) bt
> > #0  0x7fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers
> > (def=, def=, addrs=) at
> > conf/domain_addr.c:1656
> > #1  virDomainVirtioSerialAddrSetCreateFromDomain
> > (def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753
> > #2  0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses
> > (def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174
> > #3  qemuDomainAssignAddresses (def=0x7fbde81cc3f0,
> > qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0,
> > newDomain=) at qemu/qemu_domain_address.c:3174
> > #4  0x7fbe57a39e0d in virDomainDefPostParse
> > (def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20,
> > parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry=0x7fbde83ce070,
> >  parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858
> > #5  0x7fbe57a525c5 in virDomainDefParseNode (xml=,
> > root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070,
> > parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677
> > #6  0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0,
> > filename=, caps=caps@entry=0x7fbde8154d20,
> > xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0,
> >  flags=flags@entry=4610) at conf/domain_conf.c:21628
> > #7  0x7fbe57a528f6 in virDomainDefParseFile (filename= > out>, caps=caps@entry=0x7fbde8154d20,
> > xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0,
> > flags=flags@entry=4610)
> >  at conf/domain_conf.c:21653
> > #8  0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0,
> > notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070
> > "/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050
> > "/etc/libvirt/qemu",
> >  xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20, doms=0x7fbde8126940) at
> > conf/virdomainobjlist.c:503
> > #9  virDomainObjListLoadAllConfigs (doms=0x7fbde8126940,
> > configDir=0x7fbde8124050 "/etc/libvirt/qemu",
> > autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart",
> > liveStatus=liveStatus@entry=false,
> >  caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, notify=0x0, opaque=0x0)
> > at conf/virdomainobjlist.c:625
> > #10 0x7fbe017f57e2 in qemuStateInitialize (privileged=true,
> > callback=, opaque=) at
> > qemu/qemu_driver.c:1007
> > #11 0x7fbe57b8033d in virStateInitialize (privileged=true,
> > mandatory=mandatory@entry=false, callback=callback@entry=0x55dfb702ecc0
> > , opaque=opaque@entry=0x55dfb8869d60)
> >  at libvirt.c:666
> > #12 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/11/19 9:45 AM, Ján Tomko wrote:

On Mon, Oct 07, 2019 at 05:49:14PM -0400, Cole Robinson wrote:

This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.

A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.

The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html

For testing, you can create a new qcow2+raw data_file image from an
existing image, like:

   qemu-img convert -O qcow2 \
   -o data_file=NEW.raw,data_file_raw=yes
   EXISTING.raw NEW.qcow2

The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are

 * Teach storagefile how to parse out data_file from the qcow2 header
 * Store the raw string as virStorageSource->externalDataStoreRaw
 * Track that as its out virStorageSource in externalDataStore
 * dac/selinux relabel externalDataStore as needed


From libvirt's perspective, externalDataStore is conceptually pretty

close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.

This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.


Future work:
 * Exposing this in the runtime XML. We need to figure out an XML


This also belongs in the persistent XML.



Agreed


   schema. It will reuse virStorageSource obviously, but the main
   thing to figure out is probably 1) what the top element name
   should be ('dataFile' maybe?), 2) where it sits in the XML
   hierarchy (under  or under  I guess)



 maybe?


The way this code is structured, we have

src->path = FOO.qcow2
src->externalDataStore-> FOO.raw

FOO.raw contains the disk/OS contents, FOO.qcow2 just the qcow2 
metadata. If we reflect that layout in the XML, we have



  

  

  


If we called it metadataStore it sounds like the layout is inverted.


 >>  * Exposing this on the qemu -blockdev command line. Similar to how

   in the blockdev world we are explicitly putting the disk backing
   chain on the command line, we can do that for data_file too.


Historically, not being explicit on the command line and letting QEMU
do the right thing has bitten us, so yes, we have to do it for data_file
too.


Then
   like persistent  XML the user will have the power
   to overwrite the data_file location for an individual VM run.



If the point of the thin qcow2 layer is to contain the dirty bitmaps for
incremental backup then running this then you might as well use a
different metadata_file? Otherwise the metadata won't match the actual
data.



I'm not sure I follow this part, but maybe that's due to data_file 
naming mixup



OTOH, I can imagine throwing away the metadata file and starting over.



Yes this is one of the main drivers I think. That the qcow2 layer gives 
qcow2 native features like dirty bitmaps, but if it ever comes to it, 
the data is still in raw format which simplifies processing the image 
with other tools. Plus raw is less of a boogieman than qcow2 for some 
people, so I think there's some marketing opportunity behind it to say 
'see your data is still there in FOO.raw'.


There's probably cases where the user would want to ditch the top level 
layer and use that data raw layer directly, but similar to writing to a 
backing image, it invalidates the top layer, and there's no rebase 
operation for data_file AFAICT. But the persistent XML will allow 
configuring that if someone wanted it



 * Figure out how we expect ovirt/rhev to be using this at runtime.
   Possibly taking a running VM using a raw image, doing blockdev-*
   magic to pivot it to qcow2+raw data_file, so it can initiate
   incremental backup on top of a previously raw only VM?


Known issues:
 * In the qemu driver, the qcow2 image metadata is only parsed
   in -blockdev world if no  is specified in the
   persistent XML. So basically if there's a  listed,
   we never parse the qcow2 header and detect the presence of
   data_file. Fixable I'm sure but I didn't look into it much yet.


This will be fixed by introducing an XML element for it.



It's part of the fix I think. We will still need to change qemu_block.c 
logic to accomodate this in some way. Right now, whether we probe the 
qcow2 file metadata is only dependent on  in the 
persistent XML or not. But now the probing provides info on both 
backingStore and externalDataStore, so tying probing only to prescence 
of 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/10/19 11:25 PM, Han Han wrote:

Hi Cole,
I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new 
capabilities introduced by these to branches to resolve conflicts.

Then build and test as following:
# ./autogen.sh&& ./configure --without-libssh 
--build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu 
--program-prefix= --disable-dependency-tracking --prefix=/usr 
--exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin 
--sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include 
--libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var 
--sharedstatedir=/var/lib --mandir=/usr/share/man 
--infodir=/usr/share/info --with-qemu --without-openvz --without-lxc 
--without-vbox --without-libxl --with-sasl --with-polkit --with-libvirtd 
--without-phyp --with-esx --without-hyperv --without-vmware 
--without-xenapi --without-vz --without-bhyve --with-interface 
--with-network --with-storage-fs --with-storage-lvm --with-storage-iscsi 
--with-storage-iscsi-direct --with-storage-scsi --with-storage-disk 
--with-storage-mpath --with-storage-rbd --without-storage-sheepdog 
--with-storage-gluster --without-storage-zfs --without-storage-vstorage 
--with-numactl --with-numad --with-capng --without-fuse --with-netcf 
--with-selinux --with-selinux-mount=/sys/fs/selinux --without-apparmor 
--without-hal --with-udev --with-yajl --with-sanlock --with-libpcap 
--with-macvtap --with-audit --with-dtrace --with-driver-modules 
--with-firewalld --with-firewalld-zone --without-wireshark-dissector 
--without-pm-utils --with-nss-plugin '--with-packager=Unknown, 
2019-08-19-12:13:01, lab.rhel8.me ' 
--with-packager-version=1.el8 --with-qemu-user=qemu 
--with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM 
--enable-werror --enable-expensive-tests --with-init-script=systemd 
--without-login-shell && make


Start libvirtd and virtlogd
# LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
# LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd

Then try to list all domains:
# virsh list --all

Libvirtd exits with segment fault:
[1]    30104 segmentation fault (core dumped)  LD_PRELOAD="$(find src 
-name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd


Version:
qemu-4.1

Backtrace:
(gdb) bt
#0  0x7fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers 
(def=, def=, addrs=) at 
conf/domain_addr.c:1656
#1  virDomainVirtioSerialAddrSetCreateFromDomain 
(def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753
#2  0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses 
(def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174
#3  qemuDomainAssignAddresses (def=0x7fbde81cc3f0, 
qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0, 
newDomain=) at qemu/qemu_domain_address.c:3174
#4  0x7fbe57a39e0d in virDomainDefPostParse 
(def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20, 
parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry=0x7fbde83ce070,

     parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858
#5  0x7fbe57a525c5 in virDomainDefParseNode (xml=, 
root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, 
parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677
#6  0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0, 
filename=, caps=caps@entry=0x7fbde8154d20, 
xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0,

     flags=flags@entry=4610) at conf/domain_conf.c:21628
#7  0x7fbe57a528f6 in virDomainDefParseFile (filename=out>, caps=caps@entry=0x7fbde8154d20, 
xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0, 
flags=flags@entry=4610)

     at conf/domain_conf.c:21653
#8  0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0, 
notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070 
"/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050 
"/etc/libvirt/qemu",
     xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20, doms=0x7fbde8126940) at 
conf/virdomainobjlist.c:503
#9  virDomainObjListLoadAllConfigs (doms=0x7fbde8126940, 
configDir=0x7fbde8124050 "/etc/libvirt/qemu", 
autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart", 
liveStatus=liveStatus@entry=false,
     caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, notify=0x0, opaque=0x0) 
at conf/virdomainobjlist.c:625
#10 0x7fbe017f57e2 in qemuStateInitialize (privileged=true, 
callback=, opaque=) at 
qemu/qemu_driver.c:1007
#11 0x7fbe57b8033d in virStateInitialize (privileged=true, 
mandatory=mandatory@entry=false, callback=callback@entry=0x55dfb702ecc0 
, opaque=opaque@entry=0x55dfb8869d60)

     at libvirt.c:666
#12 0x55dfb702ed1d in daemonRunStateInit (opaque=0x55dfb8869d60) at 
remote/remote_daemon.c:846
#13 0x7fbe579f4be2 in virThreadHelper (data=) at 
util/virthread.c:196
#14 0x7fbe55a322de in start_thread (arg=) at 
pthread_create.c:486
#15 0x7fbe55763133 in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:95


Could you please check this issue?
The full 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Ján Tomko]

On Mon, Oct 07, 2019 at 05:49:14PM -0400, Cole Robinson wrote:

This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.

A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.

The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html

For testing, you can create a new qcow2+raw data_file image from an
existing image, like:

   qemu-img convert -O qcow2 \
   -o data_file=NEW.raw,data_file_raw=yes
   EXISTING.raw NEW.qcow2

The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are

 * Teach storagefile how to parse out data_file from the qcow2 header
 * Store the raw string as virStorageSource->externalDataStoreRaw
 * Track that as its out virStorageSource in externalDataStore
 * dac/selinux relabel externalDataStore as needed


From libvirt's perspective, externalDataStore is conceptually pretty

close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.

This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.


Future work:
 * Exposing this in the runtime XML. We need to figure out an XML


This also belongs in the persistent XML.


   schema. It will reuse virStorageSource obviously, but the main
   thing to figure out is probably 1) what the top element name
   should be ('dataFile' maybe?), 2) where it sits in the XML
   hierarchy (under  or under  I guess)



 maybe?


 * Exposing this on the qemu -blockdev command line. Similar to how
   in the blockdev world we are explicitly putting the disk backing
   chain on the command line, we can do that for data_file too.


Historically, not being explicit on the command line and letting QEMU
do the right thing has bitten us, so yes, we have to do it for data_file
too.


Then
   like persistent  XML the user will have the power
   to overwrite the data_file location for an individual VM run.



If the point of the thin qcow2 layer is to contain the dirty bitmaps for
incremental backup then running this then you might as well use a
different metadata_file? Otherwise the metadata won't match the actual
data.

OTOH, I can imagine throwing away the metadata file and starting over.


 * Figure out how we expect ovirt/rhev to be using this at runtime.
   Possibly taking a running VM using a raw image, doing blockdev-*
   magic to pivot it to qcow2+raw data_file, so it can initiate
   incremental backup on top of a previously raw only VM?


Known issues:
 * In the qemu driver, the qcow2 image metadata is only parsed
   in -blockdev world if no  is specified in the
   persistent XML. So basically if there's a  listed,
   we never parse the qcow2 header and detect the presence of
   data_file. Fixable I'm sure but I didn't look into it much yet.


This will be fixed by introducing an XML element for it.

Jano



Most of this is cleanups and refactorings to simplify the actual
functional changes.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Michal Privoznik]

On 10/7/19 11:49 PM, Cole Robinson wrote:

This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.

A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.

The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html

For testing, you can create a new qcow2+raw data_file image from an
existing image, like:

 qemu-img convert -O qcow2 \
 -o data_file=NEW.raw,data_file_raw=yes
 EXISTING.raw NEW.qcow2

The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are

   * Teach storagefile how to parse out data_file from the qcow2 header
   * Store the raw string as virStorageSource->externalDataStoreRaw
   * Track that as its out virStorageSource in externalDataStore
   * dac/selinux relabel externalDataStore as needed


From libvirt's perspective, externalDataStore is conceptually pretty

close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.

This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.


Future work:
   * Exposing this in the runtime XML. We need to figure out an XML
 schema. It will reuse virStorageSource obviously, but the main
 thing to figure out is probably 1) what the top element name
 should be ('dataFile' maybe?), 2) where it sits in the XML
 hierarchy (under  or under  I guess)

   * Exposing this on the qemu -blockdev command line. Similar to how
 in the blockdev world we are explicitly putting the disk backing
 chain on the command line, we can do that for data_file too. Then
 like persistent  XML the user will have the power
 to overwrite the data_file location for an individual VM run.

   * Figure out how we expect ovirt/rhev to be using this at runtime.
 Possibly taking a running VM using a raw image, doing blockdev-*
 magic to pivot it to qcow2+raw data_file, so it can initiate
 incremental backup on top of a previously raw only VM?


Known issues:
   * In the qemu driver, the qcow2 image metadata is only parsed
 in -blockdev world if no  is specified in the
 persistent XML. So basically if there's a  listed,
 we never parse the qcow2 header and detect the presence of
 data_file. Fixable I'm sure but I didn't look into it much yet.

Most of this is cleanups and refactorings to simplify the actual
functional changes.

Cole Robinson (30):
   storagefile: Make GetMetadataInternal static
   storagefile: qcow1: Check for BACKING_STORE_OK
   storagefile: qcow1: Fix check for empty backing file
   storagefile: qcow1: Let qcowXGetBackingStore fill in format
   storagefile: Check version to determine if qcow2 or not
   storagefile: Drop now unused isQCow2 argument
   storagefile: Use qcowXGetBackingStore directly
   storagefile: Push 'start' into qcow2GetBackingStoreFormat
   storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetExtensions 'format' argument
   storagefile: Fix backing format \0 check
   storagefile: Add externalDataStoreRaw member
   storagefile: Parse qcow2 external data file
   storagefile: Fill in meta->externalDataStoreRaw
   storagefile: Don't access backingStoreRaw directly in
 FromBackingRelative
   storagefile: Split out virStorageSourceNewFromChild
   storagefile: Add externalDataStore member
   storagefile: Fill in meta->externalDataStore
   security: dac: Drop !parent handling in SetImageLabelInternal
   security: dac: Add is_toplevel to SetImageLabelInternal
   security: dac: Restore image label for externalDataStore
   security: dac: break out SetImageLabelRelative
   security: dac: Label externalDataStore
   security: selinux: Simplify SetImageLabelInternal
   security: selinux: Drop !parent handling in SetImageLabelInternal
   security: selinux: Add is_toplevel to SetImageLabelInternal
   security: selinux: Restore image label for externalDataStore
   security: selinux: break out SetImageLabelRelative
   security: selinux: Label externalDataStore

  src/libvirt_private.syms|   1 -
  src/security/security_dac.c |  63 +--
  src/security/security_selinux.c |  97 +++
  src/util/virstoragefile.c   | 281 
  src/util/virstoragefile.h   |  11 +-
  5 files changed, 290 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Han Han]

Hi Cole,
I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new
capabilities introduced by these to branches to resolve conflicts.
Then build and test as following:
# ./autogen.sh&& ./configure --without-libssh
--build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu
--program-prefix= --disable-dependency-tracking --prefix=/usr
--exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
--datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
--libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
--mandir=/usr/share/man --infodir=/usr/share/info --with-qemu
--without-openvz --without-lxc --without-vbox --without-libxl --with-sasl
--with-polkit --with-libvirtd --without-phyp --with-esx --without-hyperv
--without-vmware --without-xenapi --without-vz --without-bhyve
--with-interface --with-network --with-storage-fs --with-storage-lvm
--with-storage-iscsi --with-storage-iscsi-direct --with-storage-scsi
--with-storage-disk --with-storage-mpath --with-storage-rbd
--without-storage-sheepdog --with-storage-gluster --without-storage-zfs
--without-storage-vstorage --with-numactl --with-numad --with-capng
--without-fuse --with-netcf --with-selinux
--with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal
--with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap
--with-audit --with-dtrace --with-driver-modules --with-firewalld
--with-firewalld-zone --without-wireshark-dissector --without-pm-utils
--with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01,
lab.rhel8.me' --with-packager-version=1.el8 --with-qemu-user=qemu
--with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror
--enable-expensive-tests --with-init-script=systemd --without-login-shell
&& make

Start libvirtd and virtlogd
# LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd
# LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd

Then try to list all domains:
# virsh list --all

Libvirtd exits with segment fault:
[1]30104 segmentation fault (core dumped)  LD_PRELOAD="$(find src -name
'*.so.*'|tr '\n' ' ')" src/.libs/libvirtd

Version:
qemu-4.1

Backtrace:
(gdb) bt
#0  0x7fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers
(def=, def=, addrs=) at
conf/domain_addr.c:1656
#1  virDomainVirtioSerialAddrSetCreateFromDomain (def=def@entry=0x7fbde81cc3f0)
at conf/domain_addr.c:1753

#2  0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses
(def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174

#3  qemuDomainAssignAddresses (def=0x7fbde81cc3f0, qemuCaps=0x7fbde81d2210,
driver=0x7fbde8126850, obj=0x0, newDomain=) at
qemu/qemu_domain_address.c:3174
#4  0x7fbe57a39e0d in virDomainDefPostParse (def=def@entry=0x7fbde81cc3f0,
caps=caps@entry=0x7fbde8154d20, parseFlags=parseFlags@entry=4610,
xmlopt=xmlopt@entry=0x7fbde83ce070,
parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858
#5  0x7fbe57a525c5 in virDomainDefParseNode (xml=,
root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070,
parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677
#6  0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0,
filename=, caps=caps@entry=0x7fbde8154d20,
xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0,

flags=flags@entry=4610) at conf/domain_conf.c:21628
#7  0x7fbe57a528f6 in virDomainDefParseFile (filename=,
caps=caps@entry=0x7fbde8154d20, xmlopt=xmlopt@entry=0x7fbde83ce070,
parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=4610)
at conf/domain_conf.c:21653
#8  0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0,
notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070
"/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050
"/etc/libvirt/qemu",
xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20, doms=0x7fbde8126940) at
conf/virdomainobjlist.c:503
#9  virDomainObjListLoadAllConfigs (doms=0x7fbde8126940,
configDir=0x7fbde8124050 "/etc/libvirt/qemu", autostartDir=0x7fbde8124070
"/etc/libvirt/qemu/autostart", liveStatus=liveStatus@entry=false,

caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, notify=0x0, opaque=0x0) at
conf/virdomainobjlist.c:625
#10 0x7fbe017f57e2 in qemuStateInitialize (privileged=true,
callback=, opaque=) at
qemu/qemu_driver.c:1007

#11 0x7fbe57b8033d in virStateInitialize (privileged=true,
mandatory=mandatory@entry=false, callback=callback@entry=0x55dfb702ecc0
, opaque=opaque@entry=0x55dfb8869d60)
at libvirt.c:666
#12 0x55dfb702ed1d in daemonRunStateInit (opaque=0x55dfb8869d60) at
remote/remote_daemon.c:846
#13 0x7fbe579f4be2 in virThreadHelper (data=) at
util/virthread.c:196
#14 0x7fbe55a322de in start_thread (arg=) at
pthread_create.c:486
#15 0x7fbe55763133 in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Could you please check this issue?
The full threads backtrace is in attachment

On Tue, Oct 8, 2019 at 5:49 AM Cole Robinson  wrote:

> This series is the first steps to 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Daniel Henrique Barboza]

Hi,


ACKed basically everything, perhaps one or two patches I found something
worth talking about but nothing too gamebreaker. Logic-wise everything made
sense to me, but I believe someone else with a deeper understanding of the
storage backend in Libvirt might know better.

I am not sure how hard it is to test the changes you're making here, but it
would be good to have new stuff in virstoragetest.c (seems like the best 
place)

to cover this new data_file support as well.

On a side note: from patches 20+ I got an impression that I was reviewing
the same patches over and over again. I think it's a good idea to have a 
look
at the code repetition between the files in src/security dir, or at the 
very least

between security_dac.c and security_selinux.c files.


Thanks,


DHB


On 10/7/19 6:49 PM, Cole Robinson wrote:

This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.

A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.

The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html

For testing, you can create a new qcow2+raw data_file image from an
existing image, like:

 qemu-img convert -O qcow2 \
 -o data_file=NEW.raw,data_file_raw=yes
 EXISTING.raw NEW.qcow2

The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are

   * Teach storagefile how to parse out data_file from the qcow2 header
   * Store the raw string as virStorageSource->externalDataStoreRaw
   * Track that as its out virStorageSource in externalDataStore
   * dac/selinux relabel externalDataStore as needed

>From libvirt's perspective, externalDataStore is conceptually pretty
close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.

This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.


Future work:
   * Exposing this in the runtime XML. We need to figure out an XML
 schema. It will reuse virStorageSource obviously, but the main
 thing to figure out is probably 1) what the top element name
 should be ('dataFile' maybe?), 2) where it sits in the XML
 hierarchy (under  or under  I guess)

   * Exposing this on the qemu -blockdev command line. Similar to how
 in the blockdev world we are explicitly putting the disk backing
 chain on the command line, we can do that for data_file too. Then
 like persistent  XML the user will have the power
 to overwrite the data_file location for an individual VM run.

   * Figure out how we expect ovirt/rhev to be using this at runtime.
 Possibly taking a running VM using a raw image, doing blockdev-*
 magic to pivot it to qcow2+raw data_file, so it can initiate
 incremental backup on top of a previously raw only VM?


Known issues:
   * In the qemu driver, the qcow2 image metadata is only parsed
 in -blockdev world if no  is specified in the
 persistent XML. So basically if there's a  listed,
 we never parse the qcow2 header and detect the presence of
 data_file. Fixable I'm sure but I didn't look into it much yet.

Most of this is cleanups and refactorings to simplify the actual
functional changes.

Cole Robinson (30):
   storagefile: Make GetMetadataInternal static
   storagefile: qcow1: Check for BACKING_STORE_OK
   storagefile: qcow1: Fix check for empty backing file
   storagefile: qcow1: Let qcowXGetBackingStore fill in format
   storagefile: Check version to determine if qcow2 or not
   storagefile: Drop now unused isQCow2 argument
   storagefile: Use qcowXGetBackingStore directly
   storagefile: Push 'start' into qcow2GetBackingStoreFormat
   storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetExtensions 'format' argument
   storagefile: Fix backing format \0 check
   storagefile: Add externalDataStoreRaw member
   storagefile: Parse qcow2 external data file
   storagefile: Fill in meta->externalDataStoreRaw
   storagefile: Don't access backingStoreRaw directly in
 FromBackingRelative
   storagefile: Split out virStorageSourceNewFromChild
   storagefile: Add externalDataStore member
   storagefile: Fill in meta->externalDataStore
   security: dac: Drop !parent handling in SetImageLabelInternal
   security: dac: Add is_toplevel to 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Cole Robinson]

On 10/8/19 1:52 AM, Christian Ehrhardt wrote:

On Mon, Oct 7, 2019 at 11:49 PM Cole Robinson  wrote:


This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.

A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.

The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html

For testing, you can create a new qcow2+raw data_file image from an
existing image, like:

 qemu-img convert -O qcow2 \
 -o data_file=NEW.raw,data_file_raw=yes
 EXISTING.raw NEW.qcow2

The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are

   * Teach storagefile how to parse out data_file from the qcow2 header
   * Store the raw string as virStorageSource->externalDataStoreRaw
   * Track that as its out virStorageSource in externalDataStore
   * dac/selinux relabel externalDataStore as needed

>From libvirt's perspective, externalDataStore is conceptually pretty
close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.

This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.


Future work:
   * Exposing this in the runtime XML. We need to figure out an XML
 schema. It will reuse virStorageSource obviously, but the main
 thing to figure out is probably 1) what the top element name
 should be ('dataFile' maybe?), 2) where it sits in the XML
 hierarchy (under  or under  I guess)

   * Exposing this on the qemu -blockdev command line. Similar to how
 in the blockdev world we are explicitly putting the disk backing
 chain on the command line, we can do that for data_file too. Then
 like persistent  XML the user will have the power
 to overwrite the data_file location for an individual VM run.

   * Figure out how we expect ovirt/rhev to be using this at runtime.
 Possibly taking a running VM using a raw image, doing blockdev-*
 magic to pivot it to qcow2+raw data_file, so it can initiate
 incremental backup on top of a previously raw only VM?


Known issues:
   * In the qemu driver, the qcow2 image metadata is only parsed
 in -blockdev world if no  is specified in the
 persistent XML. So basically if there's a  listed,
 we never parse the qcow2 header and detect the presence of
 data_file. Fixable I'm sure but I didn't look into it much yet.

Most of this is cleanups and refactorings to simplify the actual
functional changes.

Cole Robinson (30):
   storagefile: Make GetMetadataInternal static
   storagefile: qcow1: Check for BACKING_STORE_OK
   storagefile: qcow1: Fix check for empty backing file
   storagefile: qcow1: Let qcowXGetBackingStore fill in format
   storagefile: Check version to determine if qcow2 or not
   storagefile: Drop now unused isQCow2 argument
   storagefile: Use qcowXGetBackingStore directly
   storagefile: Push 'start' into qcow2GetBackingStoreFormat
   storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetBackingStoreFormat
   storagefile: Rename qcow2GetExtensions 'format' argument
   storagefile: Fix backing format \0 check
   storagefile: Add externalDataStoreRaw member
   storagefile: Parse qcow2 external data file
   storagefile: Fill in meta->externalDataStoreRaw
   storagefile: Don't access backingStoreRaw directly in
 FromBackingRelative
   storagefile: Split out virStorageSourceNewFromChild
   storagefile: Add externalDataStore member
   storagefile: Fill in meta->externalDataStore
   security: dac: Drop !parent handling in SetImageLabelInternal
   security: dac: Add is_toplevel to SetImageLabelInternal
   security: dac: Restore image label for externalDataStore
   security: dac: break out SetImageLabelRelative
   security: dac: Label externalDataStore
   security: selinux: Simplify SetImageLabelInternal
   security: selinux: Drop !parent handling in SetImageLabelInternal
   security: selinux: Add is_toplevel to SetImageLabelInternal
   security: selinux: Restore image label for externalDataStore
   security: selinux: break out SetImageLabelRelative
   security: selinux: Label externalDataStore


Hi Cole,
it seems the changes to dac/selinux follow a common pattern, in the
past those changes then mostly applied to security-apparmor as well.
Are you going to add patches for that security backend as well before
this 

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Christian Ehrhardt]

On Mon, Oct 7, 2019 at 11:49 PM Cole Robinson  wrote:
>
> This series is the first steps to teaching libvirt about qcow2
> data_file support, aka external data files or qcow2 external metadata.
>
> A bit about the feature: it was added in qemu 4.0. It essentially
> creates a two part image file: a qcow2 layer that just tracks the
> image metadata, and a separate data file which is stores the VM
> disk contents. AFAICT the driving use case is to keep a fully coherent
> raw disk image on disk, and only use qcow2 as an intermediate metadata
> layer when necessary, for things like incremental backup support.
>
> The original qemu patch posting is here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html
>
> For testing, you can create a new qcow2+raw data_file image from an
> existing image, like:
>
> qemu-img convert -O qcow2 \
> -o data_file=NEW.raw,data_file_raw=yes
> EXISTING.raw NEW.qcow2
>
> The goal of this series is to teach libvirt enough about this case
> so that we can correctly relabel the data_file on VM startup/shutdown.
> The main functional changes are
>
>   * Teach storagefile how to parse out data_file from the qcow2 header
>   * Store the raw string as virStorageSource->externalDataStoreRaw
>   * Track that as its out virStorageSource in externalDataStore
>   * dac/selinux relabel externalDataStore as needed
>
> >From libvirt's perspective, externalDataStore is conceptually pretty
> close to a backingStore, but the main difference is its read/write
> permissions should match its parent image, rather than being readonly
> like backingStore.
>
> This series has only been tested on top of the -blockdev enablement
> series, but I don't think it actually interacts with that work at
> the moment.
>
>
> Future work:
>   * Exposing this in the runtime XML. We need to figure out an XML
> schema. It will reuse virStorageSource obviously, but the main
> thing to figure out is probably 1) what the top element name
> should be ('dataFile' maybe?), 2) where it sits in the XML
> hierarchy (under  or under  I guess)
>
>   * Exposing this on the qemu -blockdev command line. Similar to how
> in the blockdev world we are explicitly putting the disk backing
> chain on the command line, we can do that for data_file too. Then
> like persistent  XML the user will have the power
> to overwrite the data_file location for an individual VM run.
>
>   * Figure out how we expect ovirt/rhev to be using this at runtime.
> Possibly taking a running VM using a raw image, doing blockdev-*
> magic to pivot it to qcow2+raw data_file, so it can initiate
> incremental backup on top of a previously raw only VM?
>
>
> Known issues:
>   * In the qemu driver, the qcow2 image metadata is only parsed
> in -blockdev world if no  is specified in the
> persistent XML. So basically if there's a  listed,
> we never parse the qcow2 header and detect the presence of
> data_file. Fixable I'm sure but I didn't look into it much yet.
>
> Most of this is cleanups and refactorings to simplify the actual
> functional changes.
>
> Cole Robinson (30):
>   storagefile: Make GetMetadataInternal static
>   storagefile: qcow1: Check for BACKING_STORE_OK
>   storagefile: qcow1: Fix check for empty backing file
>   storagefile: qcow1: Let qcowXGetBackingStore fill in format
>   storagefile: Check version to determine if qcow2 or not
>   storagefile: Drop now unused isQCow2 argument
>   storagefile: Use qcowXGetBackingStore directly
>   storagefile: Push 'start' into qcow2GetBackingStoreFormat
>   storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
>   storagefile: Rename qcow2GetBackingStoreFormat
>   storagefile: Rename qcow2GetExtensions 'format' argument
>   storagefile: Fix backing format \0 check
>   storagefile: Add externalDataStoreRaw member
>   storagefile: Parse qcow2 external data file
>   storagefile: Fill in meta->externalDataStoreRaw
>   storagefile: Don't access backingStoreRaw directly in
> FromBackingRelative
>   storagefile: Split out virStorageSourceNewFromChild
>   storagefile: Add externalDataStore member
>   storagefile: Fill in meta->externalDataStore
>   security: dac: Drop !parent handling in SetImageLabelInternal
>   security: dac: Add is_toplevel to SetImageLabelInternal
>   security: dac: Restore image label for externalDataStore
>   security: dac: break out SetImageLabelRelative
>   security: dac: Label externalDataStore
>   security: selinux: Simplify SetImageLabelInternal
>   security: selinux: Drop !parent handling in SetImageLabelInternal
>   security: selinux: Add is_toplevel to SetImageLabelInternal
>   security: selinux: Restore image label for externalDataStore
>   security: selinux: break out SetImageLabelRelative
>   security: selinux: Label externalDataStore

Hi Cole,
it seems the changes to dac/selinux follow a common pattern, in the
past those changes then mostly applied to security-apparmor as