Add VirtIO Viomem Driver support

[kostyanf14]

@YanVugenfirer Please wait for merge untill get answer virtio-win/kvm-guest-drivers-windows#794 (comment)

[kostyanf14]

PR was rebased to current master

[akihikodaki]

Why do you need double_memory?

[akihikodaki]

Why do you need double_memory?

Nevermind. I read the help message of QEMU.

[akihikodaki]

That parsing is a bit tricky. Why don't you just convert memory into a normal JSON number so that we don't need parsing?

[kostyanf14]

Why do you need double_memory?

Nevermind. I read the help message of QEMU.

We need to set maxmemory that is greater than memory. I hope double memory is a simple variant and it will be enough.
Of course, we can do more complicated source and calc max memory based on virtio-mem-pci requested memory and memory-backend-ram.

[akihikodaki]

Why don't you always add maxmem and instead make it specific to virtio-mem-pci?

[kostyanf14]

In the case of NetKVM (3 VM), we will have the sum of maxmem greater than the host RAM on CI servers. I want to be sure that the host always has some free memory.

[akihikodaki]

Is it problematic?

[akihikodaki]

I think it's not that strange that the sum of maxmem is greater than the host RAM.

Think of a multi-tenant machine farm; there are multiple machines, and each machine hosts multiple VMs. Also, assume that you want to scale VMs bigger without downtime.
In such a scenario, you set maxmem of all machines to match the maximum memory size a VM can hold. The sum of maxmem of VMs in a machine can surpass the physical memory size. Later, if too many memory is requested for a machine, some VMs on the machine will migrate to make space.
Probably some cloud vendors already do this kind of trick.

[kostyanf14]

I don't know the QEMU source related to maxmem option. If you can confirm that QEMU will not allocate more than memory_size on Windows Guest without user action. I am ok.

[akihikodaki]

You can assume that; I believe the very purpose of maxmem is to support a scenario I have just described, and in that case, we should never have a problem with overcomitted maxmem.

[kostyanf14]

Ok. Let's assume that. We can always change this if see any problem.

[kostyanf14]

Let's merge changes separately because we are waiting with this PR until the new driver is ready.

Please review #308

[kostyanf14]

That parsing is a bit tricky. Why don't you just convert memory into a normal JSON number so that we don't need parsing?

Do you suggest to convert 4G into 4294967296?

[akihikodaki]

That parsing is a bit tricky. Why don't you just convert memory into a normal JSON number so that we don't need parsing?

Do you suggest to convert 4G into 4294967296?

No, just 4.

[kostyanf14]

That parsing is a bit tricky. Why don't you just convert memory into a normal JSON number so that we don't need parsing?

Do you suggest to convert 4G into 4294967296?

No, just 4.

And suppose that we always use GB as a unit? Maybe rename key name to memory_gb in this case?

[akihikodaki]

That parsing is a bit tricky. Why don't you just convert memory into a normal JSON number so that we don't need parsing?

Do you suggest to convert 4G into 4294967296?

No, just 4.

And suppose that we always use GB as a unit? Maybe rename key name to memory_gb in this case?

Yes. Renaming key is a nice suggestion.

[akihikodaki]

Let's have @pluggable_memory@ and @max_memory@; @max_memory@ will be the sum of @memory@ and @pluggable_memory@.

[kostyanf14]

In this case, I need to talk with the driver developer. You can see that memory-backend-ram and virtio-mem-pci have a different memory size. What is @pluggable_memory@ now 2G or 1G? That's why I don't prefer ideas with calculation.

[akihikodaki]

The same applies even now; changing the size of mem_backend into 512M will certainly cause a trouble. Having @pluggable_memory@ only moves the place where the value 2G is written, and allows calculating @max_memory@.

That said, I see no reason that request-size has to be inequal to mem_backend size.

[kostyanf14]

Just checked QE command line extra_cmd= ' -object memory-backend-ram,id=vmem0,size=5G -device virtio-mem-pci,id=mem0,memdev=vmem0,node=0,requested-size=2G,indirect_desc=off,bus='+bus_num

request-size != mem_backend size.

[akihikodaki]

It's better to ask if changing requested-size is OK.

[kostyanf14]

@akihikodaki
Let's continue with request-size != mem_backend size.

What comments do you have in this case?

[akihikodaki]

@akihikodaki Let's continue with request-size != mem_backend size.

What comments do you have in this case?

The comment in https://github.com/HCK-CI/AutoHCK/pull/143/files#r1495953291 still applies. I have nothing else to add.

[kostyanf14]

@akihikodaki Let's continue with request-size != mem_backend size.
What comments do you have in this case?

The comment in https://github.com/HCK-CI/AutoHCK/pull/143/files#r1495953291 still applies. I have nothing else to add.

How do you expect to see the syntax of the @max_memory@ calculation? Or do you expect this as the internal logic of AutoHCK if @pluggable_memory@ is defined?

[akihikodaki]

@akihikodaki Let's continue with request-size != mem_backend size.
What comments do you have in this case?

The comment in https://github.com/HCK-CI/AutoHCK/pull/143/files#r1495953291 still applies. I have nothing else to add.

How do you expect to see the syntax of the @max_memory@ calculation? Or do you expect this as the internal logic of AutoHCK if @pluggable_memory@ is defined?

I expect an internal logic.

[akihikodaki]

Why don't you make pluggable_memory_gb a number defined with platform as memory_gb is?

[kostyanf14]

Why don't you make pluggable_memory_gb a number defined with platform as memory_gb is?

I am ok with making pluggable_memory_gb a number, but it doesn't relate to a platform.
This fully depends on a device for now.

[akihikodaki]

Why don't you make pluggable_memory_gb a number defined with platform as memory_gb is?

I am ok with making pluggable_memory_gb a number, but it doesn't relate to a platform. This fully depends on a device for now.

I don't think pluggable_memory_gb is that different from memory_gb. The platform provides memory, and the devices and tests use it. Tests would just fail if the provided memory is not sufficient. We can use the same logic for pluggable memory as well. But It's not a strong opinion and I won't insist.

In any case, don't put it in define_variables. It is actually a list of string replacements and better to be typed as T::Hash[String, String]. Instead, put pluggable_memory_gb at the root of the device JSON.