On Tue, Jul 07, 2020 at 03:50:09PM -0400, Peter Xu wrote: > On Mon, Jul 06, 2020 at 11:17:32PM -0700, Sean Christopherson wrote: > > On Fri, Jul 03, 2020 at 02:41:22PM -0400, Peter Xu wrote: > > > On Thu, Jul 02, 2020 at 04:08:49PM -0700, Sean Christopherson wrote: > > > > This technically needs to set as_id in the deleted memslot. I highly > > > > doubt > > > > it will ever matter from a functionality perspective, but it'd be > > > > confusing > > > > to encounter a memslot whose as_id did not match that of its owner. > > > > > > Yeah it shouldn't matter because as_id is directly passed in to look up > > > the > > > pointer of kvm_memslots in kvm_delete_memslot, and memslot->as_id > > > shouldn't be > > > further referenced. > > > > > > I can add a comment above if this can clarify things a bit: > > > > > > + u16 as_id; /* cache of as_id; only valid if npages != 0 */ > > > > Why not just set it? > > Because the value is useless even if set? :)
It's useless when things go according to plan, but I can see it being useful if there's a bug that leads to consumption of a deleted memslot. Maybe not "useful" so much as "not misleading". > You mean in kvm_delete_memslot(), am I right? Yes. > > It's a single line of code, and there's more than one > > "shouldn't" in the above. > > If you want, I can both set it and add the comment. Thanks, Why bother with the comment? It'd be wrong in the sense that the as_id is always valid/accurate, even if npages == 0.
