Re: [PATCH] Install to LVM PVs

[Dryden Personalis]

Andrei Borzenkov schreef op 09-05-2016 8:07:


I'm sorry, this email had gone missing from my system. I just found it 
back in the trash, after thinking I had already searched for it.

I'm not sure why no one responds anymore, of course Andrei was the only 
one to respond in the first place. I hope nothing is the matter with 
Andrei himself, I mean it could be personal circumstances.

I have basically implemented all suggestions, with just two questions 
that I would like to have answered:

- is it okay to drop the requirement of having a VG configured in the PV
- is it okay to skip the FS-presence test in a LVM that cannot have it 
in any case?

If the group or the ones in charge of this software would be willing to 
shed their light on these issues, I could at least make something that 
you would agree with, if it would be intended for real submission.

So from my perspective the feature works, I tend to think these are the 
only choices to make, and perhaps additional safety checks are required 
to deal with the size of the bootloaderarea, which I can look into if 
needed.

Presently I do not have the knowledge about the PV structure yet.

I did not write the original patch, after all.

I could easily test or demonstrate what would happen if the 
bootloaderareasize were really small; whether grub would overwrite 
essential structures if I did.

I think safety is the biggest requirement here?

As demonstrated, booting itself is not an issue except that I cannot 
test installing on partitions (at all) currently. I do not know why, but 
Grub on my system will not install on any partition whatsoever, not the 
new version, or the standard ubuntu version.

I have still not installed my system again on that harddisk, still 
working off of a USB stick. ;-). So I am actually waiting with 
installing until people have indicated whether additional development or 
testing is required?

I would hope to work with you on this.

(Not just grub, I'm also worried about LVM thin safety, but that is 
another issue).

I'm sorry if I seem not to understand how things work around here, or if 
I actually do not (understand). I have tried to reach Andrei off-list, 
but no response. But I've waited for a week, so I need to go and start 
doing something else if it is not going to happen anyway.

So please.

And regards, and thanks for cooperating (thus far) and of having been of 
help, in any case, regardless. Regards.



> 08.05.2016 16:01, Dryden Personalis пишет:
> > > > -    if (fs_probe)
> > > > +    if (!is_lvm && fs_probe)
> > >
> > > No, we want to probe for FS here to eliminate corner case of both PV 
> > > and
> > > FS metadata.
> >
> > You mean you want to change it to:
>
> No, I do not want to change it at all. It should remain as is and 
> always
> probe for FS so that test below actually works
> ...
> > > And you need to add check for LVM here
> > >
> > >     if (ctx.multiple_partmaps || (ctx.dest_partmap && fs) || (is_ldm
> > > && fs))
> >
> > I take it you mean:
> >
> > if (!is_lvm && (ctx.multiple_partmaps || (ctx.dest_partmap && fs) ||
> > (is_ldm && fs)))
>
> I mean
>
> if (ctx.multiple_partmaps || (ctx.dest_partmap && fs) || ((is_ldm ||
> is_lvm) && fs))