Re: [PATCH] hw/nvme: Add options to override hardcoded values

[Mauricio Sandt]

I want to argue the other way around. Why shouldn't those values
be tunable by the user? You are right; if misconfigured, it could 
potentially
break stuff on the driver side, but unless you manually set values for model
and firmware, the default is used (just like it is now), so this patch
wouldn't break any existing setups.
In my opinion, having more freedom in the values being used for model names
and similar is much better than hardcoding values. The previously hardcoded
values should remain the default if no custom value is provided.
My specific use case that required this patch is a piece of malware that 
used
several IOCTLs to read model, firmware, and nqn from the NVMe attached to
the VM. Modifying that info at the hypervisor level was a much better 
approach
than loading an (unsigned) driver into windows to intercept said IOCTLs.
Having this patch in the official qemu version would help me a lot in my 
test
lab, and I'm pretty sure it would also help other people.

I guess there could be a warning about potential problems with drivers 
in the
description for model, firmware, and nqn, but I haven't experienced any 
issues
when changing the values (for all of them). If you encountered any problems,
how did they manifest?

On 13/07/2022 19:03, Keith Busch wrote:
> On Sun, Jun 12, 2022 at 12:35:09AM +0200, Mauricio Sandt wrote:
> > This small patch is the result of some recent malware research I did
> > in a QEMU VM. The malware used multiple ways of querying info from
> > the VM disk and I needed a clean way to change those values from the
> > hypervisor.
> >
> > I believe this functionality could be useful to more people from multiple
> > fields, sometimes you just want to change some default values and having
> > them hardcoded in the sourcecode makes that much harder.
> >
> > This patch adds three config parameters to the nvme device, all of them
> > are optional to not break anything. If any of them are not specified,
> > the previous (before this patch) default is used.
> >
> > -model - This takes a string and sets it as the devices model name.
> > If you don't specify this parameter, the default is "QEMU NVMe Ctrl".
> >
> > -firmware - The firmware version string, max 8 ascii characters.
> > The default is whatever `QEMU_VERSION` evaluates to.
> >
> > -nqn_override - Allows to set a custom nqn on the nvme device.
> > Only used if there is no subsystem. This string should be in the same
> > format as the default "nqn.2019-08.org.qemu:...", but there is no
> > validation for that. Its up to the user to provide a valid string.
> I guess the nqn can be user tunable just like it is when used with subsystems,
> but what's the point of messing with model and firmware? That could mess with
> host drivers' ability to detect what quirks it needs to apply to specific
> instances of this virtual controller.