Re: [libvirt] [PATCHv2 4/5] storage: don't follow backing chain symlinks too eagerly

On 02/15/13 21:38, Eric Blake wrote: > If you have a qcow2 file /path1/to/file pointed to by symlink > /path2/symlink, and pass qemu /path2/symlink, then qemu treats > a relative backing file in the qcow2 metadata as being relative > to /path2, not /path1/to. Yes, this means that it is possible > to create a qcow2 file where the choice of WHICH directory and > symlink you access its contents from will then determine WHICH > backing file (if any) you actually find; the results can be > rather screwy, but we have to match what qemu does. > > Libvirt and qemu default to creating absolute backing file > names, so most users don't hit this. But at least VDSM uses > symlinks and relative backing names alongside the > --reuse-external flags to libvirt snapshot operations, with the > result that libvirt was failing to follow the intended chain of > backing files, and then backing files were not granted the > necessary sVirt permissions to be opened by qemu. > > See https://bugzilla.redhat.com/show_bug.cgi?id=903248 for > more gory details. This fixes a regression introduced in > commit 8250783. > > I tested this patch by creating the following chain:

> + } else if (!d_len) { I'd go for (d_len == 0) here so that it will not be confused with pointers.

> + start = "."; > + d_len = 1; > + } > + if (virAsprintf(&combined, "%.*s/%s", (int)d_len, start, > path) < 0) { > + virReportOOMError(); > + goto cleanup; > + } > } Otherwise looks reasonable. ACK.