On 10/14/19 11:59 AM, Alwin Antreich wrote: > On Mon, Oct 14, 2019 at 11:44:59AM +0200, Thomas Lamprecht wrote: >> On 10/11/19 1:45 PM, Alwin Antreich wrote: >>> On Fri, Oct 11, 2019 at 12:17:28PM +0200, Thomas Lamprecht wrote: >>>> On 10/11/19 12:02 PM, Alwin Antreich wrote: >>>>> On Fri, Oct 11, 2019 at 07:10:53AM +0200, Thomas Lamprecht wrote: >>>>>> On 10/10/19 3:58 PM, Alwin Antreich wrote: >>>>>>> Machine states that were created on snapshots with memory could not be >>>>>>> restored on rollback. The state volume was not activated so KVM couldn't >>>>>>> load the state. >>>>>>> >>>>>>> This patch removes the path generation on rollback. It uses the vmstate >>>>>>> and de-/activates the state volume in vm_start. This in turn disallows >>>>>>> the use of path based statefiles when used with the '--stateuri' option >>>>>>> on 'qm start'. Only 'tcp', 'unix' and our storage based URIs can be >> >> this is also API breakage, or? Why not a simple path check fallback in >> cfg2cmd? > That's what I am not sure about, see my earlier email. Should I check > for file/device paths or just drop it? > https://pve.proxmox.com/pipermail/pve-devel/2019-October/039465.html >
I wouldn't just drop it, even if it's just a edge case and does not affect many users (if even some)I mean if it worked before it should keep doing so. A simple "if (-e $vmstate)" could be enough and cheap to check.. >>> Do you mean by "correct place", the else clause with the "--loadstate"? >>> It can't go there because the config_to_command has to happen before, as >>> it assigns the @$cmd. The other options are then pushed in addition to the >>> @$cmd, if the statefile is equal to tcp or unix. >>> >> >> but we could move that below? >> Do a @extra_cmd (or the like) and have then one unified statefile handling? >> I mean this method is already very big and the more things inside are spread >> out the harder it's to maintain it.. > Yes, I will go ahead and put this into v3. I mean it was rather an open question, .. not 100% sure it's the best thing to do (just FYI) > >> >> Also this makes your "It uses the vmstate and de-/activates the state volume >> in vm_start" sentence from the commit message false, as it's activated in >> config_to_command not vm_start ... > True, I will rewrite the commit message in v3 to also better reflect the > outcome of the discussion. > thanks! _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
