[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] qemuDomainGetPreservedMounts: Fix suffixes for corner cases




On 06/15/2017 04:53 AM, Michal Privoznik wrote:
> On 06/14/2017 09:58 PM, John Ferlan wrote:
>>
>>
>> On 06/12/2017 11:57 AM, Michal Privoznik wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1431112
>>>
>>> Imagine a FS mounted on /dev/blah/blah2. Our process of creating
>>> suffix for temporary location where all the mounted filesystems
>>> are moved is very simplistic. We want:
>>>
>>> /var/run/libvirt/qemu/$domName.$suffix\
>>>
>>> were $suffix is just the mount point path stripped of the "/dev/"
>>> preffix. For instance:
>>
>> s/preffix/prefix
>>
>>>
>>> /var/run/libvirt/qemu/fedora.mqueue  for /dev/mqueue
>>> /var/run/libvirt/qemu/fedora.pts     for /dev/pts
>>>
>>> and so on. Now if we plug /dev/blah/blah2 into the example we see
>>> some misbehaviour:
>>>
>>> /var/run/libvirt/qemu/fedora.blah/blah2
>>>
>>> Well, misbehaviour if /dev/blah/blah2 is a file, because in that
>>> case we call virFileTouch() instead of virFileMakePath().
>>>
>>
>> You didn't finish my bedtime story!
>>
>> Am I to assume that instead of :
>>
>> /var/run/libvirt/qemu/fedora.blah/blah2
>>
>> we would get
>>
>> /var/run/libvirt/qemu/fedora.blah.blah2
> 
> Yes.
> 
>>
>> taking things one step further...
>>
>> would /dev/blah/blah2/blah3
>>
>> be
>>
>> /var/run/libvirt/qemu/fedora.blah.blah2.blah3
> 
> 
> Yes.
> 
>>
>> That's what I see coded at least... Or should the path be:
>>
>> /var/run/libvirt/qemu/fedora.blah/blah2.blah3
> 
> 
> Nope. The former one.
> 
>>
>>
>> It would seem you'd want to get to the end, reverse search on '/' then
>> if that spot is greater than @off, then convert it to a '.',
> 
> So basically, this is my approach just reversed. What'd be the benefits?
> I find my algorithm small and easy to understand.
> 
>> but what do
>> I know. I keep to the simple life and don't use namespaces.
> 
> Well, until a7cc039dc I didn't know that you can bind mount files. What
> a strange thing to learn. What I want to say - you can learn some new
> stuff when using namespaces ;-)
> 
> Michal
> 

OK fair enough - be sure to finish the bed time story that this patch
converts @suffix directory 'layers' into the flat namespace using '.'
instead of '/'.  The whole comment for 'mounts[i] is ...' could be
simplified to indicate that we're turning the complete @suffix from a
possible multi-directory level into a single flat file reference.

Reviewed-by: John Ferlan <jferlan redhat com>

Just so you know IDC if you keep the for (...) in patch 2, I'm just not
a fan. You're the author, you got my OK for your method even though it's
not my personal preference.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]