<vmid>.conf comments?

[tin]

I see that the "Notes" section is recorded within the conf files by using a # symbol like it's a comment. Is this the only comment symbol that can be used in <vmid>.conf files, or is there another that is treated as not being a "note"?

I have several containers that use external hardware and custom ID mapping. I would really like to add comments to these files that don't appear in the GUI notes, but do appear in the config, so that I can have notes about specific config lines, not just about the machine.

 

[dcsapak]

no there is currently no additional comment syntax for vm/ct config files

 

[yarcod]

I would like to necro this old thread because it's basically the only relevant question I've found on these forums.

Is there really still no way to make inline comments in the .conf files? I'm using Ansible (blockinfile) to add mounts to a LXC configuration, but the wrapping comment section is constantly moved to the top which breaks the Ansible functionality. Could this be disabled with an option somewhere in Proxmox/CLI?

 

[leesteken]

While they are at it, could the mangling of :-characters be disable for comments? Really annoying when commenting out PCIe passthrough.

 

[fiona]

Hi,

I would like to necro this old thread because it's basically the only relevant question I've found on these forums.

Is there really still no way to make inline comments in the .conf files? I'm using Ansible (blockinfile) to add mounts to a LXC configuration, but the wrapping comment section is constantly moved to the top which breaks the Ansible functionality. Could this be disabled with an option somewhere in Proxmox/CLI?

in general, it's not a good idea to work directly with the text file. You should rather use the API or CLI interface, which is stable and does checks for validity.

While they are at it, could the mangling of :-characters be disable for comments? Really annoying when commenting out PCIe passthrough.

We can't do that, because we need to stay compatible with the existing format. There is an enhancement request about being able to disable parts of the hardware/configuration.

 

[yarcod]

In general, it's not a good idea to work directly with the text file. You should rather use the API or CLI interface, which is stable and does checks for validity.

I can understand that you'd rather have me use the CLI interface, but in case of rbind there is currently no support with your own notation. Instead I have to use the config file by itself and add it manually.

One idea would be to provide two kinds of notes/comments in the file -- one that supports being inlined and one that is marked as a note and moved to the top, e.g. '##' and '#' respectively. That should not break current behaviour while still providing a way to have comments in the file when needed.

 

[fiona]

I can understand that you'd rather have me use the CLI interface, but in case of rbind there is currently no support with your own notation. Instead I have to use the config file by itself and add it manually.

I think there were some security concerns in the past with recursive bind mount, not sure what the current status is. There is an enhancement request with initial patch, but it didn't get applied.

One idea would be to provide two kinds of notes/comments in the file -- one that supports being inlined and one that is marked as a note and moved to the top, e.g. '##' and '#' respectively. That should not break current behaviour while still providing a way to have comments in the file when needed.

The issue is that that everything loading/modifying/writing with the config would need to remember where each comment should go and I'm not sure that's worth the effort.

 

[yarcod]

I think there were some security concerns in the past with recursive bind mount, not sure what the current status is. There is an enhancement request with initial patch, but it didn't get applied.

Ah, I can't say that I am qualified to determine if there are any security implications, but from the little I can find the basis is the same as regular mount, only that it is performed multiple times to include bindable subdirectories. For future googlers, a description of rbind can be found in section 5c) of: https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt

The issue is that that everything loading/modifying/writing with the config would need to remember where each comment should go and I'm not sure that's worth the effort.

I'm sure you know theses parts of the code much more than I could ever hope, and if this is a big issue then I guess it would be a larger fix than anticipated. But if the configuration lines are pulled out one by one, wouldn't it be possible to simply leave the commented ones in place and not touch them? Not saying that I know anything about how the configuration is loaded (could you provide a link? )

 

[fiona]

I'm sure you know theses parts of the code much more than I could ever hope, and if this is a big issue then I guess it would be a larger fix than anticipated. But if the configuration lines are pulled out one by one, wouldn't it be possible to simply leave the commented ones in place and not touch them? Not saying that I know anything about how the configuration is loaded (could you provide a link? )

It's stored as a Perl hash, so the key-value-pairs are pulled out. When written, the keys are ordered lexicographically. With comments, you'd need key-value-comment triples. Doesn't sound like a big change in theory, but in practice it'd mean adapting everything that touches the hash, because it has a different structure now. For example, the code of the parser.

 

[yarcod]

It's stored as a Perl hash, so the key-value-pairs are pulled out. When written, the keys are ordered lexicographically. With comments, you'd need key-value-comment triples. Doesn't sound like a big change in theory, but in practice it'd mean adapting everything that touches the hash, because it has a different structure now. For example, the code of the parser.

I've never written Perl -- I have barely read Perl either for that matter -- but I spent just a few minutes giving it a quick try. And again, not saying that I know anything about the rest of the code base, there could be multiple pitfalls here, of course, but would something like this work? https://pastebin.com/RraCbhKK
Basically the idea was to add a field to the configuration you mentioned which associates each inline comment(s) with an actual configuration line and reinserts the comment to the file when that key:value pair is being written back. By storing it as a separate config parameter, the hope was that nothing else needing the actual config line would be affected. I've neither run nor tested any of it, I just gave it a quick look in hopes of finding a solution

 

[t.lamprecht]

Since Proxmox VE 7.0 the Notes panel is rendered as markdown, which means you can simply use markdown/html like comments for hiding stuff in the panel. Note that the user could still check with the web developer console or the API call response, or simply by editing the node again, so it shouldn't be used for secret info, just for "avoid overloading/cramp the note field visually"

<!-- This is commented out. -->

You can even use the GUI to add such comments.

We don't plan to add any other comment field that is treaded differently/internally.

 

[yarcod]

Since Proxmox VE 7.0 the Notes panel is rendered as markdown, which means you can simply use markdown/html like comments for hiding stuff in the panel. Note that the user could still check with the web developer console or the API call response, or simply by editing the node again, so it shouldn't be used for secret info, just for "avoid overloading/cramp the note field visually"

<!-- This is commented out. -->

You can even use the GUI to add such comments.

Ah, I see. I like that you implemented markdown support in the notes panel -- that makes it more useful! However, I'm not sure that it works for my use case, since I want to control the mounts via a separate tool and not just keep notes about it. You mentioned API -- is there a way to edit the .conf file though the API instead of directly? pct commands are not enough since they fall back to the mpX format.

We don't plan to add any other comment field that is treaded differently/internally.

That's too bad. Even so, could you check my link and see if that is a viable solution, or at least a start? I get what @fiona said about it being too much work if a lot of code had to be re-written, but if what I have done does not affect anything else then this "feature" is more or less for free?

 

[fiona]

Ah, I see. I like that you implemented markdown support in the notes panel -- that makes it more useful! However, I'm not sure that it works for my use case, since I want to control the mounts via a separate tool and not just keep notes about it. You mentioned API -- is there a way to edit the .conf file though the API instead of directly? pct commands are not enough since they fall back to the mpX format.

The real issue is that recursive bind mount is currently not officially supported by us. I'm afraid you'll have to wait until the feature request is implemented or use a solution without inline comments (why not work with mp0,mp1,etc. in the text file directly?).

That's too bad. Even so, could you check my link and see if that is a viable solution, or at least a start? I get what @fiona said about it being too much work if a lot of code had to be re-written, but if what I have done does not affect anything else then this "feature" is more or less for free?

Well, by factoring-out all necessary information into a separate hash key, your solution does avoid those changes I mentioned. The downside is that you'd loose the comment as soon as the config line changes (e.g. resizing a disk). Could be avoided by associating only the config key with the comment. And I think you'd still need to adapt relevant parts of backup/restore code, but as @t.lamprecht said, we don't currently plan to implement this. And it's not for free, because functionality/code needs to be maintained

 

[yarcod]

The real issue is that recursive bind mount is currently not officially supported by us. I'm afraid you'll have to wait until the feature request is implemented or use a solution without inline comments (why not work with mp0,mp1,etc. in the text file directly?).

Thanks, then I know what to expect. I would have no problem using mp0 etc., but as far as I have understood it this notation does not support rbind as of yet? Or is the feature request about something else? I'm trying to attach ZFS datasets to LXC containers, and get permission problems when trying to access underlying datasets if not mounted with rbind.

Well, by factoring-out all necessary information into a separate hash key, your solution does avoid those changes I mentioned. The downside is that you'd loose the comment as soon as the config line changes (e.g. resizing a disk). Could be avoided by associating only the config key with the comment. And I think you'd still need to adapt relevant parts of backup/restore code, but as @t.lamprecht said, we don't currently plan to implement this. And it's not for free, because functionality/code needs to be maintained

Thanks for taking a look and for the explanation! I understand that you don't plan to add it; my hope was that if the code was mostly in place it was no longer a matter of prioritizing it but wanting to take in the feature. And of course, it's not entirely free, but it would be less time spent by you on something that you yourself do not see as beneficial. If you by chance would like me to take a look at the backup and restore parts of the config in the near future (as well as fix the issue you mentioned) just let me know -- I would very much like to have functionality in place!

 

[fiona]

Thanks, then I know what to expect. I would have no problem using mp0 etc., but as far as I have understood it this notation does not support rbind as of yet? Or is the feature request about something else? I'm trying to attach ZFS datasets to LXC containers, and get permission problems when trying to access underlying datasets if not mounted with rbind.

I'm not talking about using the API where the rbind option is not yet supported, but directly working with the text file and relying on detecting mp0, etc. instead of using comments.

Thanks for taking a look and for the explanation! I understand that you don't plan to add it; my hope was that if the code was mostly in place it was no longer a matter of prioritizing it but wanting to take in the feature. And of course, it's not entirely free, but it would be less time spent by you on something that you yourself do not see as beneficial. If you by chance would like me to take a look at the backup and restore parts of the config in the near future (as well as fix the issue you mentioned) just let me know -- I would very much like to have functionality in place!

I mean, I didn't really look at your code, just skim over it to get the idea. We don't plan to add it, so adapting backup/restore code would be wasted work, I'm afraid.

 

[yarcod]

I'm not talking about using the API where the rbind option is not yet supported, but directly working with the text file and relying on detecting mp0, etc. instead of using comments.

All right, I'm a little lost. First, do mp0 support the use of rbind? I thought the only way to use rbind was to use lxc.mount.entry, but I haven't tested the former.
Second, the problem I had with Ansible was that only the blockinfile module would allow me to add several mounts in bulk and keep them updated properly, but that method relies on comments around that block in order for Ansible to keep track of it. I tried other modules as well, but I didn't get them to work properly -- I needed to replace the existing mounts with new, and the replacement had trouble looping over my configs. In the end I made a three-step-rocket out of it instead, by reading existing mounts in the file, adding new if available and then removing any that was not part of my current Ansible config. It's not as beautiful, and probably more error-prone, but it works for now. (Now I just have to figure out the permissions...)

I mean, I didn't really look at your code, just skim over it to get the idea. We don't plan to add it, so adapting backup/restore code would be wasted work, I'm afraid.

Oh yes, I didn't expect you to do a full-on review, I was just happy that you actually took any time at all to look at it

 

[fiona]

All right, I'm a little lost. First, do mp0 support the use of rbind? I thought the only way to use rbind was to use lxc.mount.entry, but I haven't tested the former.

Sorry, right. That was a mistake on my part.

Second, the problem I had with Ansible was that only the blockinfile module would allow me to add several mounts in bulk and keep them updated properly, but that method relies on comments around that block in order for Ansible to keep track of it. I tried other modules as well, but I didn't get them to work properly -- I needed to replace the existing mounts with new, and the replacement had trouble looping over my configs. In the end I made a three-step-rocket out of it instead, by reading existing mounts in the file, adding new if available and then removing any that was not part of my current Ansible config. It's not as beautiful, and probably more error-prone, but it works for now. (Now I just have to figure out the permissions...)

Parsing and updating directly sounds more sensible to me than relying on comments honestly.

 

[yarcod]

Parsing and updating directly sounds more sensible to me than relying on comments honestly.

In general I would agree. The comments don't do anything really, they're added by Ansible so that it knows which part it should keep up-to-date. So for all updates it will basically do what I do manually now, but whereas I rely on finding my lines with regex, Ansible would instead be able to keep track of different pieces of configurations, e.g. features. However, since you sort the config keys, it wouldn't work for that reason either.

Sligthly related topic, but why do you sort the keys lexicographically? What's the benefit, other than easier to overview when the file is viewed?

 

[t.lamprecht]

Sligthly related topic, but why do you sort the keys lexicographically? What's the benefit, other than easier to overview when the file is viewed?

A stable sorting is useful for test (diff) and avoids random jumping of key, as otherwise we'd need to internally track the order we read them in, which is just extra work and useless, as nothing should depend on such details.

Note also that you could also track whatever info you want in a separate file, e.g., create a folder in /etc/pve to ensure you got some name space and just read/write a, e.g., <vmid>.meta file there; then you got full control over everything and do not need to depend on some internal details from PVE.

 

[jkossis]

A stable sorting is useful for test (diff) and avoids random jumping of key, as otherwise we'd need to internally track the order we read them in, which is just extra work and useless, as nothing should depend on such details.

Note also that you could also track whatever info you want in a separate file, e.g., create a folder in /etc/pve to ensure you got some name space and just read/write a, e.g., <vmid>.meta file there; then you got full control over everything and do not need to depend on some internal details from PVE.

You mentioned adding config to a separate <vm_id>.meta file that we could own, but how would this get picked up/merged with the standard config at /etc/pve/lxc/<vm_id>.conf? I couldn't seem to find an option for importing/merging in the config doumentation.