Provide ldomctl(8) better error messages when parsing ldom.conf

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Provide ldomctl(8) better error messages when parsing ldom.conf

Kurt Mosiejczuk-9
Currently ldomctl init-system will error out in many situations. A number of
them involve requesting more resources than there are available. Right now
onw just gets error messages like:

resource_id larger than max_mblocks
resource_id larger than max_guests
not enough VCPU resources available
not enough memory available

With this diff, we put out numbers for most of these.
ldomctl: requested vcpus (256) > vcpu resources (64)
ldomctl: requested memory (213674622976) > memory resources (137428467712)

The other change in here is moving max_cpus to uint64_t. max_cpus was an
int but is only compared to uint_64t. So I moved max_cpus to uint64_t.
I suppose I could have gone the other way. I don't know if anyone has
strong feelings on it.

Thoughts? Comments? ok?

--Kurt

Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v
retrieving revision 1.40
diff -u -p -r1.40 config.c
--- config.c 24 May 2020 22:08:54 -0000 1.40
+++ config.c 24 May 2020 22:10:56 -0000
@@ -122,7 +122,7 @@ void guest_fixup_phys_io(struct guest *)
 
 TAILQ_HEAD(, frag) free_frags = TAILQ_HEAD_INITIALIZER(free_frags);
 TAILQ_HEAD(, cpu) free_cpus = TAILQ_HEAD_INITIALIZER(free_cpus);
-int total_cpus;
+uint64_t total_cpus;
 TAILQ_HEAD(, mblock) free_memory = TAILQ_HEAD_INITIALIZER(free_memory);
 uint64_t total_memory;
 
@@ -601,7 +601,8 @@ hvmd_init_mblock(struct md *md, struct m
  errx(1, "missing resource_id property in mblock node");
 
  if (resource_id >= max_mblocks)
- errx(1, "resource_id larger than max_mblocks");
+ errx(1, "resource_id %lld larger than max_mblocks %lld", \
+    resource_id, max_mblocks);
 
  mblock = xzalloc(sizeof(*mblock));
  md_get_prop_val(md, node, "membase", &mblock->membase);
@@ -632,7 +633,8 @@ hvmd_init_console(struct md *md, struct
  errx(1, "missing resource_id property in console node");
 
  if (resource_id >= max_guests)
- errx(1, "resource_id larger than max_guests");
+ errx(1, "resource_id %lld larger than max_guests %lld", \
+    resource_id, max_guests);
 
  console = xzalloc(sizeof(*console));
  md_get_prop_val(md, node, "ino", &console->ino);
@@ -655,7 +657,8 @@ hvmd_init_cpu(struct md *md, struct md_n
  errx(1, "missing resource_id property in cpu node");
 
  if (resource_id >= max_cpus)
- errx(1, "resource_id larger than max-cpus");
+ errx(1, "resource_id %lld larger than max-cpus %lld", \
+    resource_id, max_cpus);
 
  if (!md_get_prop_val(md, node, "pid", &pid))
  errx(1, "missing pid property in cpu node");
@@ -698,7 +701,8 @@ hvmd_init_device(struct md *md, struct m
  errx(1, "missing resource_id property in ldc_endpoint node");
 
  if (resource_id >= max_devices)
- errx(1, "resource_id larger than max_devices");
+ errx(1, "resource_id %lld larger than max_devices %lld", \
+    resource_id, max_devices);
 
  device = xzalloc(sizeof(*device));
  md_get_prop_val(md, node, "gid", &device->gid);
@@ -754,7 +758,8 @@ hvmd_init_endpoint(struct md *md, struct
  errx(1, "missing resource_id property in ldc_endpoint node");
 
  if (resource_id >= max_guest_ldcs)
- errx(1, "resource_id larger than max_guest_ldcs");
+ errx(1, "resource_id %lld larger than max_guest_ldcs %lld", \
+    resource_id, max_guest_ldcs);
 
  if (ldc_endpoints[resource_id]) {
  /*
@@ -800,7 +805,8 @@ hvmd_init_guest(struct md *md, struct md
  errx(1, "missing resource_id property in guest node");
 
  if (resource_id >= max_guests)
- errx(1, "resource_id larger than max_guests");
+ errx(1, "resource_id %lld larger than max_guests %lld", \
+    resource_id, max_guests);
 
  guest = xzalloc(sizeof(*guest));
  TAILQ_INIT(&guest->cpu_list);
@@ -2816,10 +2822,19 @@ build_config(const char *filename, int n
  primary_num_cpus = total_cpus - num_cpus;
  if (primary_memory == 0 && total_memory > memory)
  primary_memory = total_memory - memory;
- if (num_cpus > total_cpus || primary_num_cpus == 0)
- errx(1, "not enough VCPU resources available");
- if (memory > total_memory || primary_memory == 0)
- errx(1, "not enough memory available");
+ if (num_cpus > total_cpus)
+ errx(1, "requested vcpus (%lld) > vcpu resources (%lld)", \
+    num_cpus, total_cpus);
+ if (primary_num_cpus == 0)
+ errx(1, "No vcpus left for primary domain");
+ if (memory > total_memory)
+ errx(1, "requested memory (%lld) > memory resources (%lld)", \
+    memory, total_memory);
+ if (primary_memory == 0)
+ errx(1, "No memory left for primary domain");
+
+ if (noaction)
+ exit(0);
 
  if (noaction)
  exit(0);

Reply | Threaded
Open this post in threaded view
|

Re: Provide ldomctl(8) better error messages when parsing ldom.conf

Mark Kettenis
> Date: Sun, 24 May 2020 18:59:09 -0400
> From: Kurt Mosiejczuk <[hidden email]>
>
> Currently ldomctl init-system will error out in many situations. A number of
> them involve requesting more resources than there are available. Right now
> onw just gets error messages like:
>
> resource_id larger than max_mblocks
> resource_id larger than max_guests
> not enough VCPU resources available
> not enough memory available
>
> With this diff, we put out numbers for most of these.
> ldomctl: requested vcpus (256) > vcpu resources (64)
> ldomctl: requested memory (213674622976) > memory resources (137428467712)
>
> The other change in here is moving max_cpus to uint64_t. max_cpus was an
> int but is only compared to uint_64t. So I moved max_cpus to uint64_t.
> I suppose I could have gone the other way. I don't know if anyone has
> strong feelings on it.
>
> Thoughts? Comments? ok?

Including numbers is good!  I feel, quite strongly, that lowercase
vcpu isn't right.  So please use VCPU (or plural VCPUs) instead.  I
also think that your proposal is a bit inconsistent regarding > and
"larger than".  So:

ldomctl: requested VCPUs (256) larger than available resources (64)
ldomctl: requested memory (213674622976) larger than availableresources (137428467712)

Now that last message is a bit too long and I think those numbers look
a bit silly, so better would be:

ldomctl: requested memory (203776M) larger than available resources (131062M)

I think MB is the right unit.  you could avaid a silly x > x by
rounding the requested memory up to the nearest MB.

Cheers,

Mark


> Index: config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 config.c
> --- config.c 24 May 2020 22:08:54 -0000 1.40
> +++ config.c 24 May 2020 22:10:56 -0000
> @@ -122,7 +122,7 @@ void guest_fixup_phys_io(struct guest *)
>  
>  TAILQ_HEAD(, frag) free_frags = TAILQ_HEAD_INITIALIZER(free_frags);
>  TAILQ_HEAD(, cpu) free_cpus = TAILQ_HEAD_INITIALIZER(free_cpus);
> -int total_cpus;
> +uint64_t total_cpus;
>  TAILQ_HEAD(, mblock) free_memory = TAILQ_HEAD_INITIALIZER(free_memory);
>  uint64_t total_memory;
>  
> @@ -601,7 +601,8 @@ hvmd_init_mblock(struct md *md, struct m
>   errx(1, "missing resource_id property in mblock node");
>  
>   if (resource_id >= max_mblocks)
> - errx(1, "resource_id larger than max_mblocks");
> + errx(1, "resource_id %lld larger than max_mblocks %lld", \
> +    resource_id, max_mblocks);
>  
>   mblock = xzalloc(sizeof(*mblock));
>   md_get_prop_val(md, node, "membase", &mblock->membase);
> @@ -632,7 +633,8 @@ hvmd_init_console(struct md *md, struct
>   errx(1, "missing resource_id property in console node");
>  
>   if (resource_id >= max_guests)
> - errx(1, "resource_id larger than max_guests");
> + errx(1, "resource_id %lld larger than max_guests %lld", \
> +    resource_id, max_guests);
>  
>   console = xzalloc(sizeof(*console));
>   md_get_prop_val(md, node, "ino", &console->ino);
> @@ -655,7 +657,8 @@ hvmd_init_cpu(struct md *md, struct md_n
>   errx(1, "missing resource_id property in cpu node");
>  
>   if (resource_id >= max_cpus)
> - errx(1, "resource_id larger than max-cpus");
> + errx(1, "resource_id %lld larger than max-cpus %lld", \
> +    resource_id, max_cpus);
>  
>   if (!md_get_prop_val(md, node, "pid", &pid))
>   errx(1, "missing pid property in cpu node");
> @@ -698,7 +701,8 @@ hvmd_init_device(struct md *md, struct m
>   errx(1, "missing resource_id property in ldc_endpoint node");
>  
>   if (resource_id >= max_devices)
> - errx(1, "resource_id larger than max_devices");
> + errx(1, "resource_id %lld larger than max_devices %lld", \
> +    resource_id, max_devices);
>  
>   device = xzalloc(sizeof(*device));
>   md_get_prop_val(md, node, "gid", &device->gid);
> @@ -754,7 +758,8 @@ hvmd_init_endpoint(struct md *md, struct
>   errx(1, "missing resource_id property in ldc_endpoint node");
>  
>   if (resource_id >= max_guest_ldcs)
> - errx(1, "resource_id larger than max_guest_ldcs");
> + errx(1, "resource_id %lld larger than max_guest_ldcs %lld", \
> +    resource_id, max_guest_ldcs);
>  
>   if (ldc_endpoints[resource_id]) {
>   /*
> @@ -800,7 +805,8 @@ hvmd_init_guest(struct md *md, struct md
>   errx(1, "missing resource_id property in guest node");
>  
>   if (resource_id >= max_guests)
> - errx(1, "resource_id larger than max_guests");
> + errx(1, "resource_id %lld larger than max_guests %lld", \
> +    resource_id, max_guests);
>  
>   guest = xzalloc(sizeof(*guest));
>   TAILQ_INIT(&guest->cpu_list);
> @@ -2816,10 +2822,19 @@ build_config(const char *filename, int n
>   primary_num_cpus = total_cpus - num_cpus;
>   if (primary_memory == 0 && total_memory > memory)
>   primary_memory = total_memory - memory;
> - if (num_cpus > total_cpus || primary_num_cpus == 0)
> - errx(1, "not enough VCPU resources available");
> - if (memory > total_memory || primary_memory == 0)
> - errx(1, "not enough memory available");
> + if (num_cpus > total_cpus)
> + errx(1, "requested vcpus (%lld) > vcpu resources (%lld)", \
> +    num_cpus, total_cpus);
> + if (primary_num_cpus == 0)
> + errx(1, "No vcpus left for primary domain");
> + if (memory > total_memory)
> + errx(1, "requested memory (%lld) > memory resources (%lld)", \
> +    memory, total_memory);
> + if (primary_memory == 0)
> + errx(1, "No memory left for primary domain");
> +
> + if (noaction)
> + exit(0);
>  
>   if (noaction)
>   exit(0);
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Provide ldomctl(8) better error messages when parsing ldom.conf

Klemens Nanni-2
On Mon, May 25, 2020 at 05:55:39PM +0200, Mark Kettenis wrote:
> Including numbers is good!  I feel, quite strongly, that lowercase
> vcpu isn't right.  So please use VCPU (or plural VCPUs) instead.  I
> also think that your proposal is a bit inconsistent regarding > and
> "larger than".  So:
>
> ldomctl: requested VCPUs (256) larger than available resources (64)
> ldomctl: requested memory (213674622976) larger than availableresources (137428467712)
I concur.

> Now that last message is a bit too long and I think those numbers look
> a bit silly, so better would be:
>
> ldomctl: requested memory (203776M) larger than available resources (131062M)
>
> I think MB is the right unit.  you could avaid a silly x > x by
> rounding the requested memory up to the nearest MB.
MB is the smallest common unit;  also, as far as I can remember the
specs, the hypervisor requires memory for domains to be somewhat aligned,
e.g., a multiple of 128M or so;  that is to say any "odd" amount of
memory is unlikely to work anyway and printing errors with MB seems the
most sanest approach.

I also mentioned to kmos that we could make the error messages more
user friendly: "resource_id larger than max_guests" might be grasped,
but "resource_id larger than max_ldcs", etc. only seem confusing unless
you know what logical domain channels are.

So these should probably say something along the lines of
"Too many domains defined, maximum is <max_guests>" or so.